There’s a FreeBSD commit to telnet. fix a couple of snprintf() buffer overflows. It’s received a bit of attention for various reasons, telnet in 2019?, etc. I thought I’d take a look. Here’s a few random observations.
Here are three new lines, after the patch.
unsigned int buflen = strlen(hbuf) + strlen(cp2) + 1;
cp = (char *)malloc(sizeof(char)*buflen);
snprintf((char *)cp, buflen, "%s%s", hbuf, cp2);
1. The first line is indented with spaces while the others use tabs.
2. The correct type for string length is
sizeof(char) is always one. There’s no need to multiply by it.
4. If you do need to multiply by a size, this is an unsafe pattern. Use
calloc or something similar. (OpenBSD provides
reallocarray to avoid zeroing cost of calloc.)
5. Return value of
malloc doesn’t need to be cast. In fact, should not be, lest you disguise a warning.
6. Return value of
malloc is not checked for NULL.
7. No reason to cast
char * when passing to
snprintf. It already is that type. And if it weren’t, what are you doing?
8. The whole operation could be simplified by using
9. Although unlikely (probably impossible here, but more generally), adding the two source lengths together can overflow, resulting in truncation with an unchecked
asprintf avoids this failure case.
Posted 11 Jul 2019 04:13 by tedu Updated: 11 Jul 2019 04:13
Tagged: c programming