nqminds / edgesec

Secure router - reference implementation
https://edgesec.info
MIT License
6 stars 1 forks source link

test(runctl): fix radius_server_data heap overflow #514

Closed aloisklink closed 1 year ago

aloisklink commented 1 year ago

Fix a heap overflow error in test_run_engine(), when we weren't allocating enough bytes for a struct radius_server_data.


We may want to consider adding __attribute__((alloc_size (1)) to the os_zalloc() function to tell the compiler that it returns the number of bytes given by the size parameter.

I think compilers/static analysers should hopefully warn us then about CWE-131: Incorrect Calculation of Buffer Size.

For example, GCC has -Wanalyzer-allocation-size that would warn us.

codecov[bot] commented 1 year ago

Codecov Report

Merging #514 (aa87544) into main (76dfb38) will increase coverage by 0.98%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #514      +/-   ##
==========================================
+ Coverage   53.51%   54.49%   +0.98%     
==========================================
  Files         144      144              
  Lines       19999    20549     +550     
==========================================
+ Hits        10702    11198     +496     
- Misses       9297     9351      +54     
Impacted Files Coverage Δ
tests/test_runctl.c 94.91% <100.00%> (ø)

... and 4 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

aloisklink commented 1 year ago

Merging #514 (aa87544) into main (76dfb38) will increase coverage by 0.98%.

Looks like code-coverage has greatly increased too (a 1% increase is a lot!! We've only increased by 2.6% in the last 6 months!)

I'm guessing that the this heap overflow bug caused a bunch of test_runctl.c tests to not run correctly!