Fix string operation truncation #285
Conversation
Change-Id: I8f99658d599400113c7c215a401aea01da28aec9
Change-Id: Id7f150d4ac6b5229716c646f970a2a5c14a5ede5
| @@ -9,6 +9,6 @@ char *web_recv(int fd, struct sockaddr_in *clientaddr); | |||
|
|
|||
| void web_send(int out_fd, char *buffer); | |||
|
|
|||
| int web_eventmux(char *buf); | |||
| int web_eventmux(char *buf, size_t buflen); | |||
There was a problem hiding this comment.
If you want to change the function prototype, there must be a good reason.
There was a problem hiding this comment.
The original callback doesn't take the length of the buffer. This may result buffer overflow.
There was a problem hiding this comment.
Also, the compiler reports stringop-truncation warning for this, and stop compilation with -O3.
|
Don't the titles describe the change? The first is an obvious mistake that the variable name is wrong, and the third fixes the buffer overflow, as the commit messages say. I suppose the reason of the overflow is also obvious because a buffer always has a finite length? |
No, show the details and discuss the impact within git commit messages. Always make the development verbose and informative. |
This changes the parameter list of the eventmux callback, affecting the header and the implementation of the dependency linenoise, but it is necessary to prevent potential buffer overflow and to make the program compile with '-O3'. Change-Id: Ic50650107e939677a286f129251c50ce2dcbf635
lumynou5
force-pushed
the
fix-stringop-truncation
branch
from
cb85746 to
a3539f1
Compare
The buffer length passed to the callback is the actual length minus 1, so 'buflen' is not out-of-bounds but exactly the last byte of the buffer. Change-Id: I63a2bb1928c80f5c78dac7133133ad2c89f82e5f
|
Thank @lumynou5 for contributing! |
This patch fixes potential truncation caused by
strncpy. If the destination buffer was not large enough, the string would be not null-terminated, and possibly lead to buffer overflow because of the passed buffer length depending on the source buffer.