{"action":"created","issue":{"url":"https://api.github.com/repos/NTIA/ehata/issues/5","repository_url":"https://api.github.com/repos/NTIA/ehata","labels_url":"https://api.github.com/repos/NTIA/ehata/issues/5/labels{/name}","comments_url":"https://api.github.com/repos/NTIA/ehata/issues/5/comments","events_url":"https://api.github.com/repos/NTIA/ehata/issues/5/events","html_url":"https://github.com/NTIA/ehata/issues/5","id":365002031,"node_id":"MDU6SXNzdWUzNjUwMDIwMzE=","number":5,"title":"Application crash if profile step < 25m","user":{"login":"sbdt-google","id":30304613,"node_id":"MDQ6VXNlcjMwMzA0NjEz","avatar_url":"https://avatars2.githubusercontent.com/u/30304613?v=4","gravatar_id":"","url":"https://api.github.com/users/sbdt-google","html_url":"https://github.com/sbdt-google","followers_url":"https://api.github.com/users/sbdt-google/followers","following_url":"https://api.github.com/users/sbdt-google/following{/other_user}","gists_url":"https://api.github.com/users/sbdt-google/gists{/gist_id}","starred_url":"https://api.github.com/users/sbdt-google/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/sbdt-google/subscriptions","organizations_url":"https://api.github.com/users/sbdt-google/orgs","repos_url":"https://api.github.com/users/sbdt-google/repos","events_url":"https://api.github.com/users/sbdt-google/events{/privacy}","received_events_url":"https://api.github.com/users/sbdt-google/received_events","type":"User","site_admin":false},"labels":[],"state":"closed","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":0,"created_at":"2018-09-28T18:52:11Z","updated_at":"2018-10-04T05:00:49Z","closed_at":"2018-10-04T04:51:10Z","author_association":"NONE","body":"A fixed array of size 400 is allocated on the stack in PreprocessTerrainPath.cpp, routine 'ComputeTerrainStatistics'.\r\n```\r\n   float pfl_segment[400];\r\n```\r\nWhere it holds up to 10km of path.\r\nSo if the step between profile points is below 25m, a buffer overrun will happen.\r\n\r\nPossible simple solution are either:\r\n - do not allow step smaller than 25m, in the entry point of the model\r\n - or dynamically allocate the array. This is the approach taken by Winnforum\r\n```\r\n    float *pfl_segment = new float[i_end - i_start + 2];\r\n   ...\r\n    delete[] plf_segment;\r\n```\r\nSee https://github.com/Wireless-Innovation-Forum/Spectrum-Access-System/blob/master/src/harness/reference_models/propagation/ehata/its/PreprocessTerrainPath.cpp#L176\r\n\r\nNote that some other alternatives could be:\r\n - use of `std::vector pfl_segment(i_end - i_start+2);` : requires to use the C++ standard library, so the code is no more pure-C compatible\r\n - variable stack array, if your compiler supports it (moder ones do): `float pfl_segment[i_end-i_start + 2];`\r\n\r\nThere is the same issue in function 'MobileTerrainSlope()' with array:\r\n```\r\nfloat pfl_segment[400] = { 0 };\r\n```\r\nIn Winnforum we did:\r\n```\r\n    float *pfl_segment = new float[int(10000/xi) + 4]();\r\n   ...\r\n    delete[] pfl_segment;\r\n```\r\nalthough the other option could be done as well"},"comment":{"url":"https://api.github.com/repos/NTIA/ehata/issues/comments/426885942","html_url":"https://github.com/NTIA/ehata/issues/5#issuecomment-426885942","issue_url":"https://api.github.com/repos/NTIA/ehata/issues/5","id":426885942,"node_id":"MDEyOklzc3VlQ29tbWVudDQyNjg4NTk0Mg==","user":{"login":"wkozmaNTIA","id":25177540,"node_id":"MDQ6VXNlcjI1MTc3NTQw","avatar_url":"https://avatars2.githubusercontent.com/u/25177540?v=4","gravatar_id":"","url":"https://api.github.com/users/wkozmaNTIA","html_url":"https://github.com/wkozmaNTIA","followers_url":"https://api.github.com/users/wkozmaNTIA/followers","following_url":"https://api.github.com/users/wkozmaNTIA/following{/other_user}","gists_url":"https://api.github.com/users/wkozmaNTIA/gists{/gist_id}","starred_url":"https://api.github.com/users/wkozmaNTIA/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/wkozmaNTIA/subscriptions","organizations_url":"https://api.github.com/users/wkozmaNTIA/orgs","repos_url":"https://api.github.com/users/wkozmaNTIA/repos","events_url":"https://api.github.com/users/wkozmaNTIA/events{/privacy}","received_events_url":"https://api.github.com/users/wkozmaNTIA/received_events","type":"User","site_admin":false},"created_at":"2018-10-04T05:00:49Z","updated_at":"2018-10-04T05:00:49Z","author_association":"COLLABORATOR","body":"For clarification, this change was made, but the array size is one smaller than commented above, for the following reason:\r\n\r\nIn ComputeTerrainStatistics(),\r\n\r\n```\r\n  0   1   2   3   4   5   6   7\r\n---------------------------------\r\n|np |xi |   |   |   |   |   |   |\r\n---------------------------------\r\n```\r\n\r\nSay you want to copy out elements 2 through 6, ie,\r\ni_start = 2\r\ni_end = 6\r\n\r\nfor total of 5 elements.  Thus,\r\n\r\ndouble *pfl_segment = new double[i_end - i_start + 1];\r\n\r\nsince pfl_segement doesn't contain np and xi.\r\n\r\nIn MobileTerrainSlope(), you include np and xi, so its 'new double[i_end - i_start + 3]'"}}
{"actor":{"display_login":"wkozmaNTIA"}}