-
-
Notifications
You must be signed in to change notification settings - Fork 6
refactor: Replace unsafe strcpy with safer alternatives and fix build warnings #419
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
JFreegman
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@JFreegman made 2 comments.
Reviewable status: 1 change requests, 0 of 1 approvals obtained (waiting on @iphydf).
src/misc_tools.c line 491 at r2 (raw file):
on_error: snprintf(buf, TOXIC_MAX_NAME_LENGTH + 1, "%s", UNKNOWN_NAME);
The buffer size here (and in all the ones below) is TOX_MAX_NAME_LENGTH which is incidentally <= than TOXIC_MAX_NAME_LENGTH. Maybe we should pass the buffer size so there's less room for error.
src/misc_tools.c line 694 at r2 (raw file):
if (len > MAX_WINDOW_NAME_LENGTH) { snprintf(&cpy[MAX_WINDOW_NAME_LENGTH - 3], 4, "...");
Why 4 instead of sizeof(cpy)?
iphydf
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@iphydf made 2 comments.
Reviewable status: 1 change requests, 0 of 1 approvals obtained (waiting on @JFreegman).
src/misc_tools.c line 491 at r2 (raw file):
Previously, JFreegman wrote…
The buffer size here (and in all the ones below) is
TOX_MAX_NAME_LENGTHwhich is incidentally <= thanTOXIC_MAX_NAME_LENGTH. Maybe we should pass the buffer size so there's less room for error.
Done.
src/misc_tools.c line 694 at r2 (raw file):
Previously, JFreegman wrote…
Why
4instead ofsizeof(cpy)?
Done.
bda3613 to
df3b5a5
Compare
…ings. Replaces unsafe `strcpy` calls with `snprintf`, `memcpy`, or `strdup` to mitigate potential buffer overflows. Also fixed `-Wdouble-promotion` build warnings in `netprof.c` and `prompt.c` by using explicit `double` casts and `sprintf` in `configdir.c`.
JFreegman
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@JFreegman reviewed 16 files and all commit messages, and resolved 2 discussions.
Reviewable status:complete! 1 of 1 approvals obtained.
Replaces unsafe
strcpycalls withsnprintf,memcpy, orstrdupto mitigate potential buffer overflows. Also fixed-Wdouble-promotionbuild warnings innetprof.candprompt.cby using explicitdoublecasts andsprintfinconfigdir.c.This change is