Skip to content
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

Various safety improvements #55

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

jackrosenthal
Copy link

Three patches are included which replace functions which are commonly a cause of buffer overflows.

Read the commit message of each patch for description.

sprintf is not a safe function to call: it will potentially write
beyond the end of a buffer.  It can be replaced with snprintf to
prevent buffer overflow.
tolower can return the input, which is potentially signed.  Saving as
an unsigned char will prevent a compiler warning (Below from GCC
12.2):

reg.c: In function ‘reg_putraw’:
reg.c:21:18: warning: array subscript [-128, 255] is outside array bounds of ‘char *[256]’ [-Warray-bounds]
   21 |         free(bufs[tolower(c)]);
      |              ~~~~^~~~~~~~~~~~
reg.c:6:14: note: while referencing ‘bufs’
    6 | static char *bufs[256];
      |              ^~~~
reg.c:22:13: warning: array subscript [-128, 255] is outside array bounds of ‘char *[256]’ [-Warray-bounds]
   22 |         bufs[tolower(c)] = buf;
      |         ~~~~^~~~~~~~~~~~
reg.c:6:14: note: while referencing ‘bufs’
    6 | static char *bufs[256];
      |              ^~~~
strcpy() does not check the size of the destination buffer, and may
cause buffer overflows.  Replace usages of strcpy with safer code.
@jackrosenthal
Copy link
Author

what's it take to get this merged?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant