Here's a summary of the changes in this patch: * Fix buffer overrun when using strncpy() * Use startswith() macro instead strncmp() * Use strcmp() instead of strncmp() where appropriate * Use const char * where appropriate * Use size_t instead of unsigned int in readl().
The startswith() macro is debatable -- it won't hurt my feelings if no
one likes it. It also adds 2 lines to sic.c, bringing the total to
more than 250. ;)
Also, parsesrv() should be using strcmp() on cmd, not strncmp(), but I
decided not to try to fix that in this patch.
The only real problem is the buffer overrun after using strncpy().
Since it happens only when reading 'nick' or 'channel', I could have
fixed it in another way, by continuing to treat the contents of these
buffers as non-null-terminated char arrays instead of strings, so for
example, this call:
snprintf(bufout, sizeof bufout,
"PASS %s\r\nNICK %s\r\nUSER %s localhost %s :%s\r\n",
password, nick, nick, host, nick);
would be rewritten as:
snprintf(bufout, sizeof bufout,
"PASS %s\r\nNICK %.*s\r\nUSER %.*s localhost %s :%.*s\r\n",
password, sizeof nick, nick, sizeof nick, nick, host, sizeof
nick, nick);
or maybe just this:
snprintf(bufout, sizeof bufout,
"PASS %s\r\nNICK %.32s\r\nUSER %.32s localhost %s :%.32s\r\n",
password, nick, nick, host, nick);
and similarly for other places where 'nick' and 'channel' are read. I
decided against this. The way it is fixed in this patch relies on the
fact that these arrays are default-initialized to all '\0' bytes and
also that the last byte in each (nick[31] and channel[255]) is never
written to, thus the contents are always null-terminated. There's
also the data truncation issue (what if the server actually issues a
NICK change longer than 31 characters to you?) which is probably OK to
continue to ignore in this case.
I have more changes, including getting rid of calls to strlen(),
fixing the blocking I/O issues, and fixing the IRC protocol parsing in
parsesrv(). The cumulative line count delta for these changes is
actually negative (down to 245 lines). I should probably just attach
these now too, but I want to start with this one.
-Mark
sic-1.0-strings.diff
Description: Binary data
