search

darrencroton / sage

Home of the Semi-Analytic Galaxy Evolution (SAGE) galaxy formation model
MIT License
21 stars 21 forks source link

Reduce chance of string buffer overflow exploits #17

Open broukema opened 3 years ago

broukema commented 3 years ago

Gcc-10.2.0 gives many compile warnings of the risk of string buffer overflows. The uncontrolled sprintf commands risk segmentation violations if long path lengths are used, and risk security problems more generally, which is why the warnings exist.

This commit aims to reduce the chance of buffer overflows by using snprintf instead of sprintf and extending from just the C preprocesor macro MAX_STRING_LEN to extra macros MAX_BUF_SIZE and MAX_OUTFILE_SIZE, defined e.g. as (3*MAX_STRING_LEN+40). A few unused variables are also commented out in response to the recommendations from gcc.

This commit compiles cleanly for me with gcc-10.2.0, with make OPTIONS=-fcommon to work around Issue https://github.com/darrencroton/sage/issues/16 .

darrencroton commented 3 years ago

Thanks broukema. Let me have a look.

broukema commented 3 years ago

Darren, my name is actually "Boud" (lower case is fine in this context, of course - boud) :), not broukema. The broukema alias is only because someone else than me grabbed 'boud' on github before I did... :P

@darrencroton

manodeep commented 3 years ago

While I said "request changes", really the only necessary change is the first one

manodeep commented 3 years ago

And many thanks for this PR and improving the code quality!

broukema commented 3 years ago

@manodeep So I see that "conversation" is another non-git service that github provides on top of git and risks creating vendor lock-in, because it's not easy to shift to other git repository hosters, and is nevertheless useful for people who wish to trace what happened, when and why in order to debug or tidy up code.

Anyway, I've updated this pull request. Thanks for the review! :)