snprintf() truncation checks were one byte too conservative.

This commit is contained in:
Nicholas J. Kain 2022-02-25 06:11:16 -05:00
parent ba8f674e15
commit 7572e2eb8b
10 changed files with 72 additions and 109 deletions

2
cfg.c
View File

@ -5940,7 +5940,7 @@ void parse_cmdline(int argc, char *argv[])
0, argv[i]);
else
snl = snprintf(argb + argbl, sizeof argb - argbl, "%s", argv[i]);
if (snl < 0 || (size_t)snl >= sizeof argb)
if (snl < 0 || (size_t)snl > sizeof argb)
suicide("error parsing command line option: option too long");
argbl += (size_t)snl;
}

2
cfg.rl
View File

@ -328,7 +328,7 @@ void parse_cmdline(int argc, char *argv[])
0, argv[i]);
else
snl = snprintf(argb + argbl, sizeof argb - argbl, "%s", argv[i]);
if (snl < 0 || (size_t)snl >= sizeof argb)
if (snl < 0 || (size_t)snl > sizeof argb)
suicide("error parsing command line option: option too long");
argbl += (size_t)snl;
}

View File

@ -19,11 +19,8 @@
static void get_duid_path(char *duidfile, size_t dlen)
{
int splen = snprintf(duidfile, dlen, "%s/DUID", state_dir);
if (splen < 0)
if (splen < 0 || (size_t)splen > dlen)
suicide("%s: snprintf failed; return=%d", __func__, splen);
if ((size_t)splen >= dlen)
suicide("%s: snprintf dest buffer too small %d >= %zu",
__func__, splen, sizeof dlen);
}
static void get_iaid_path(char *iaidfile, size_t ilen,
@ -37,11 +34,8 @@ static void get_iaid_path(char *iaidfile, size_t ilen,
"%s/IAID-%2.2x:%2.2x:%2.2x:%2.2x:%2.2x:%2.2x",
state_dir, hwaddr[0], hwaddr[1], hwaddr[2],
hwaddr[3], hwaddr[4], hwaddr[5]);
if (splen < 0)
if (splen < 0 || (size_t)splen > ilen)
suicide("%s: snprintf failed; return=%d", __func__, splen);
if ((size_t)splen >= ilen)
suicide("%s: snprintf dest buffer too small %d >= %zu",
__func__, splen, sizeof ilen);
}
static int open_duidfile_read(void)

View File

@ -42,7 +42,7 @@ static int ifcmd_raw(char *buf, size_t buflen, const char *optname,
int ioptlen = (int)optlen;
ssize_t olen = snprintf(buf, buflen, "%s:%.*s;",
optname, ioptlen, optdata);
if (olen < 0 || (size_t)olen >= buflen) {
if (olen < 0 || (size_t)olen > buflen) {
log_line("%s: (%s) '%s' option would truncate, so it was dropped.",
client_config.interface, __func__, optname);
memset(buf, 0, buflen);
@ -65,7 +65,7 @@ static int ifcmd_u8(char *buf, size_t buflen, const char *optname,
char numbuf[16];
uint8_t c = optdata[0];
ssize_t olen = snprintf(numbuf, sizeof numbuf, "%c", c);
if (olen < 0 || (size_t)olen >= sizeof numbuf)
if (olen < 0 || (size_t)olen > sizeof numbuf)
return -1;
return ifcmd_raw(buf, buflen, optname, numbuf, strlen(numbuf));
}
@ -80,7 +80,7 @@ static int ifcmd_u16(char *buf, size_t buflen, const char *optname,
memcpy(&v, optdata, 2);
v = ntohs(v);
ssize_t olen = snprintf(numbuf, sizeof numbuf, "%hu", v);
if (olen < 0 || (size_t)olen >= sizeof numbuf)
if (olen < 0 || (size_t)olen > sizeof numbuf)
return -1;
return ifcmd_raw(buf, buflen, optname, numbuf, strlen(numbuf));
}
@ -95,7 +95,7 @@ static int ifcmd_s32(char *buf, size_t buflen, const char *optname,
memcpy(&v, optdata, 4);
v = ntohl(v);
ssize_t olen = snprintf(numbuf, sizeof numbuf, "%d", v);
if (olen < 0 || (size_t)olen >= sizeof numbuf)
if (olen < 0 || (size_t)olen > sizeof numbuf)
return -1;
return ifcmd_raw(buf, buflen, optname, numbuf, strlen(numbuf));
}
@ -123,14 +123,14 @@ static int ifcmd_iplist(char *out, size_t outlen, const char *optname,
inet_ntop(AF_INET, optdata + optoff, ipbuf, sizeof ipbuf);
ssize_t wc = snprintf(buf + bufoff, sizeof buf, "%s", ipbuf);
if (wc < 0 || (size_t)wc >= sizeof buf)
if (wc < 0 || (size_t)wc > sizeof buf)
return -1;
optoff += 4;
bufoff += (size_t)wc;
while (optlen >= 4 + optoff) {
inet_ntop(AF_INET, optdata + optoff, ipbuf, sizeof ipbuf);
wc = snprintf(buf + bufoff, sizeof buf, ",%s", ipbuf);
if (wc < 0 || (size_t)wc >= sizeof buf)
if (wc < 0 || (size_t)wc > sizeof buf)
return -1;
optoff += 4;
bufoff += (size_t)wc;
@ -270,7 +270,7 @@ static size_t send_client_ip(char *out, size_t olen,
} else {
snlen = snprintf(out, olen, "ip4:%s,%s;", ip, sn);
}
if (snlen < 0 || (size_t)snlen >= olen) {
if (snlen < 0 || (size_t)snlen > olen) {
log_line("%s: (%s) ip4 command would truncate so it was dropped.",
client_config.interface, __func__);
memset(out, 0, olen);

View File

@ -1189,14 +1189,8 @@ int execute_buffer(const char *newbuf)
ssize_t buflen = snprintf(buf, sizeof buf, "%s%s", cl.ibuf, newbuf);
memset(cl.ibuf, 0, sizeof cl.ibuf);
if (buflen < 0) {
log_line("%s: (%s) snprintf1 failed; your system is broken?",
client_config.interface, __func__);
return -99;
}
if ((size_t)buflen >= sizeof buf) {
log_line("%s: (%s) input is too long for buffer",
client_config.interface, __func__);
if (buflen < 0 || (size_t)buflen > sizeof buf) {
log_line("%s: (%s) snprintf1 failed", client_config.interface, __func__);
return -99;
}
@ -1208,15 +1202,15 @@ int execute_buffer(const char *newbuf)
int cs = 0;
#line 1212 "ifchd-parse.c"
#line 1206 "ifchd-parse.c"
{
cs = (int)ifchd_parser_start;
}
#line 194 "ifchd-parse.rl"
#line 188 "ifchd-parse.rl"
#line 1220 "ifchd-parse.c"
#line 1214 "ifchd-parse.c"
{
switch ( cs ) {
case 1:
@ -1519,7 +1513,7 @@ int execute_buffer(const char *newbuf)
#line 92 "ifchd-parse.rl"
cl.state = STATE_NOTHING; }
#line 1523 "ifchd-parse.c"
#line 1517 "ifchd-parse.c"
goto _st2;
_st2:
@ -1599,7 +1593,7 @@ int execute_buffer(const char *newbuf)
#line 155 "ifchd-parse.rl"
cl.state = STATE_CARRIER; }
#line 1603 "ifchd-parse.c"
#line 1597 "ifchd-parse.c"
{
#line 105 "ifchd-parse.rl"
@ -1630,7 +1624,7 @@ int execute_buffer(const char *newbuf)
cmdf |= pr;
}
#line 1634 "ifchd-parse.c"
#line 1628 "ifchd-parse.c"
goto _st126;
_ctr39:
@ -1647,7 +1641,7 @@ int execute_buffer(const char *newbuf)
tb[arg_len] = 0;
}
#line 1651 "ifchd-parse.c"
#line 1645 "ifchd-parse.c"
{
#line 105 "ifchd-parse.rl"
@ -1678,7 +1672,7 @@ int execute_buffer(const char *newbuf)
cmdf |= pr;
}
#line 1682 "ifchd-parse.c"
#line 1676 "ifchd-parse.c"
goto _st126;
_st126:
@ -1724,7 +1718,7 @@ int execute_buffer(const char *newbuf)
#line 92 "ifchd-parse.rl"
cl.state = STATE_NOTHING; }
#line 1728 "ifchd-parse.c"
#line 1722 "ifchd-parse.c"
goto _st10;
_st10:
@ -1773,13 +1767,13 @@ int execute_buffer(const char *newbuf)
#line 144 "ifchd-parse.rl"
cl.state = STATE_DNS; }
#line 1777 "ifchd-parse.c"
#line 1771 "ifchd-parse.c"
{
#line 93 "ifchd-parse.rl"
arg_start = p; }
#line 1783 "ifchd-parse.c"
#line 1777 "ifchd-parse.c"
goto _st14;
_ctr115:
@ -1787,13 +1781,13 @@ int execute_buffer(const char *newbuf)
#line 145 "ifchd-parse.rl"
cl.state = STATE_LPRSVR; }
#line 1791 "ifchd-parse.c"
#line 1785 "ifchd-parse.c"
{
#line 93 "ifchd-parse.rl"
arg_start = p; }
#line 1797 "ifchd-parse.c"
#line 1791 "ifchd-parse.c"
goto _st14;
_ctr126:
@ -1801,13 +1795,13 @@ int execute_buffer(const char *newbuf)
#line 146 "ifchd-parse.rl"
cl.state = STATE_NTPSVR; }
#line 1805 "ifchd-parse.c"
#line 1799 "ifchd-parse.c"
{
#line 93 "ifchd-parse.rl"
arg_start = p; }
#line 1811 "ifchd-parse.c"
#line 1805 "ifchd-parse.c"
goto _st14;
_ctr148:
@ -1815,13 +1809,13 @@ int execute_buffer(const char *newbuf)
#line 147 "ifchd-parse.rl"
cl.state = STATE_WINS; }
#line 1819 "ifchd-parse.c"
#line 1813 "ifchd-parse.c"
{
#line 93 "ifchd-parse.rl"
arg_start = p; }
#line 1825 "ifchd-parse.c"
#line 1819 "ifchd-parse.c"
goto _st14;
_st14:
@ -2044,13 +2038,13 @@ int execute_buffer(const char *newbuf)
#line 150 "ifchd-parse.rl"
cl.state = STATE_DOMAIN; }
#line 2048 "ifchd-parse.c"
#line 2042 "ifchd-parse.c"
{
#line 93 "ifchd-parse.rl"
arg_start = p; }
#line 2054 "ifchd-parse.c"
#line 2048 "ifchd-parse.c"
goto _st33;
_ctr53:
@ -2058,13 +2052,13 @@ int execute_buffer(const char *newbuf)
#line 149 "ifchd-parse.rl"
cl.state = STATE_HOSTNAME; }
#line 2062 "ifchd-parse.c"
#line 2056 "ifchd-parse.c"
{
#line 93 "ifchd-parse.rl"
arg_start = p; }
#line 2068 "ifchd-parse.c"
#line 2062 "ifchd-parse.c"
goto _st33;
_st33:
@ -2086,7 +2080,7 @@ int execute_buffer(const char *newbuf)
#line 92 "ifchd-parse.rl"
cl.state = STATE_NOTHING; }
#line 2090 "ifchd-parse.c"
#line 2084 "ifchd-parse.c"
goto _st34;
_st34:
@ -2144,7 +2138,7 @@ int execute_buffer(const char *newbuf)
#line 92 "ifchd-parse.rl"
cl.state = STATE_NOTHING; }
#line 2148 "ifchd-parse.c"
#line 2142 "ifchd-parse.c"
goto _st39;
_st39:
@ -2193,13 +2187,13 @@ int execute_buffer(const char *newbuf)
#line 143 "ifchd-parse.rl"
cl.state = STATE_IP4SET; }
#line 2197 "ifchd-parse.c"
#line 2191 "ifchd-parse.c"
{
#line 93 "ifchd-parse.rl"
arg_start = p; }
#line 2203 "ifchd-parse.c"
#line 2197 "ifchd-parse.c"
goto _st43;
_st43:
@ -2380,13 +2374,13 @@ int execute_buffer(const char *newbuf)
#line 142 "ifchd-parse.rl"
cl.state = STATE_ROUTER; }
#line 2384 "ifchd-parse.c"
#line 2378 "ifchd-parse.c"
{
#line 93 "ifchd-parse.rl"
arg_start = p; }
#line 2390 "ifchd-parse.c"
#line 2384 "ifchd-parse.c"
goto _st59;
_st59:
@ -2481,13 +2475,13 @@ int execute_buffer(const char *newbuf)
#line 154 "ifchd-parse.rl"
cl.state = STATE_IPTTL; }
#line 2485 "ifchd-parse.c"
#line 2479 "ifchd-parse.c"
{
#line 93 "ifchd-parse.rl"
arg_start = p; }
#line 2491 "ifchd-parse.c"
#line 2485 "ifchd-parse.c"
goto _st67;
_st67:
@ -2778,7 +2772,7 @@ int execute_buffer(const char *newbuf)
#line 92 "ifchd-parse.rl"
cl.state = STATE_NOTHING; }
#line 2782 "ifchd-parse.c"
#line 2776 "ifchd-parse.c"
goto _st94;
_st94:
@ -2822,7 +2816,7 @@ int execute_buffer(const char *newbuf)
#line 92 "ifchd-parse.rl"
cl.state = STATE_NOTHING; }
#line 2826 "ifchd-parse.c"
#line 2820 "ifchd-parse.c"
goto _st98;
_st98:
@ -2863,13 +2857,13 @@ int execute_buffer(const char *newbuf)
#line 153 "ifchd-parse.rl"
cl.state = STATE_MTU; }
#line 2867 "ifchd-parse.c"
#line 2861 "ifchd-parse.c"
{
#line 93 "ifchd-parse.rl"
arg_start = p; }
#line 2873 "ifchd-parse.c"
#line 2867 "ifchd-parse.c"
goto _st102;
_st102:
@ -2883,7 +2877,7 @@ int execute_buffer(const char *newbuf)
#line 92 "ifchd-parse.rl"
cl.state = STATE_NOTHING; }
#line 2887 "ifchd-parse.c"
#line 2881 "ifchd-parse.c"
goto _st103;
_st103:
@ -2927,7 +2921,7 @@ int execute_buffer(const char *newbuf)
#line 92 "ifchd-parse.rl"
cl.state = STATE_NOTHING; }
#line 2931 "ifchd-parse.c"
#line 2925 "ifchd-parse.c"
goto _st107;
_st107:
@ -2989,7 +2983,7 @@ int execute_buffer(const char *newbuf)
#line 92 "ifchd-parse.rl"
cl.state = STATE_NOTHING; }
#line 2993 "ifchd-parse.c"
#line 2987 "ifchd-parse.c"
goto _st113;
_st113:
@ -3048,13 +3042,13 @@ int execute_buffer(const char *newbuf)
#line 152 "ifchd-parse.rl"
cl.state = STATE_TIMEZONE; }
#line 3052 "ifchd-parse.c"
#line 3046 "ifchd-parse.c"
{
#line 93 "ifchd-parse.rl"
arg_start = p; }
#line 3058 "ifchd-parse.c"
#line 3052 "ifchd-parse.c"
goto _st119;
_st119:
@ -3074,7 +3068,7 @@ int execute_buffer(const char *newbuf)
#line 92 "ifchd-parse.rl"
cl.state = STATE_NOTHING; }
#line 3078 "ifchd-parse.c"
#line 3072 "ifchd-parse.c"
goto _st121;
_st121:
@ -3252,7 +3246,7 @@ int execute_buffer(const char *newbuf)
_out: {}
}
#line 195 "ifchd-parse.rl"
#line 189 "ifchd-parse.rl"
if (cs == ifchd_parser_error) {
@ -3263,14 +3257,8 @@ int execute_buffer(const char *newbuf)
if (cmd_start != pe) {
ssize_t ilen = snprintf(cl.ibuf, sizeof cl.ibuf, "%s", cmd_start);
if (ilen < 0) {
log_line("%s: (%s) snprintf2 failed; your system is broken?",
client_config.interface, __func__);
return -99;
}
if ((size_t)ilen >= sizeof buf) {
log_line("%s: (%s) unconsumed input too long for buffer",
client_config.interface, __func__);
if (ilen < 0 || (size_t)ilen > sizeof buf) {
log_line("%s: (%s) snprintf2 failed", client_config.interface, __func__);
return -99;
}
}

View File

@ -173,14 +173,8 @@ int execute_buffer(const char *newbuf)
ssize_t buflen = snprintf(buf, sizeof buf, "%s%s", cl.ibuf, newbuf);
memset(cl.ibuf, 0, sizeof cl.ibuf);
if (buflen < 0) {
log_line("%s: (%s) snprintf1 failed; your system is broken?",
client_config.interface, __func__);
return -99;
}
if ((size_t)buflen >= sizeof buf) {
log_line("%s: (%s) input is too long for buffer",
client_config.interface, __func__);
if (buflen < 0 || (size_t)buflen > sizeof buf) {
log_line("%s: (%s) snprintf1 failed", client_config.interface, __func__);
return -99;
}
@ -202,14 +196,8 @@ int execute_buffer(const char *newbuf)
if (cmd_start != pe) {
ssize_t ilen = snprintf(cl.ibuf, sizeof cl.ibuf, "%s", cmd_start);
if (ilen < 0) {
log_line("%s: (%s) snprintf2 failed; your system is broken?",
client_config.interface, __func__);
return -99;
}
if ((size_t)ilen >= sizeof buf) {
log_line("%s: (%s) unconsumed input too long for buffer",
client_config.interface, __func__);
if (ilen < 0 || (size_t)ilen > sizeof buf) {
log_line("%s: (%s) snprintf2 failed", client_config.interface, __func__);
return -99;
}
}

18
ifchd.c
View File

@ -104,7 +104,7 @@ static int write_resolve_conf(void)
else
*q++ = '\0';
ssize_t sl = snprintf(buf, sizeof buf, "%s", p);
if (sl < 0 || (size_t)sl >= sizeof buf) {
if (sl < 0 || (size_t)sl > sizeof buf) {
log_line("%s: (%s) snprintf failed appending nameservers",
client_config.interface, __func__);
}
@ -125,7 +125,7 @@ static int write_resolve_conf(void)
else
*q++ = '\0';
ssize_t sl = snprintf(buf, sizeof buf, "%s", p);
if (sl < 0 || (size_t)sl >= sizeof buf) {
if (sl < 0 || (size_t)sl > sizeof buf) {
log_line("%s: (%s) snprintf failed appending domains",
client_config.interface, __func__);
}
@ -191,9 +191,8 @@ int perform_dns(const char *str, size_t len)
return ret;
}
ssize_t sl = snprintf(cl.namesvrs, sizeof cl.namesvrs, "%s", str);
if (sl < 0 || (size_t)sl >= sizeof cl.namesvrs) {
log_line("%s: (%s) snprintf failed",
client_config.interface, __func__);
if (sl < 0 || (size_t)sl > sizeof cl.namesvrs) {
log_line("%s: (%s) snprintf failed", client_config.interface, __func__);
}
ret = write_resolve_conf();
if (ret >= 0)
@ -233,9 +232,8 @@ int perform_domain(const char *str, size_t len)
return ret;
}
ssize_t sl = snprintf(cl.domains, sizeof cl.domains, "%s", str);
if (sl < 0 || (size_t)sl >= sizeof cl.domains) {
log_line("%s: (%s) snprintf failed",
client_config.interface, __func__);
if (sl < 0 || (size_t)sl > sizeof cl.domains) {
log_line("%s: (%s) snprintf failed", client_config.interface, __func__);
}
ret = write_resolve_conf();
if (ret == 0)
@ -348,13 +346,13 @@ static void setup_resolv_conf(void)
char buf[PATH_MAX];
ssize_t sl = snprintf(buf, sizeof buf, "%s.head", resolv_conf_d);
if (sl < 0 || (size_t)sl >= sizeof buf)
if (sl < 0 || (size_t)sl > sizeof buf)
log_line("snprintf failed appending resolv_conf_head; path too long?");
else
resolv_conf_head_fd = open(buf, O_RDONLY|O_CLOEXEC, 0);
sl = snprintf(buf, sizeof buf, "%s.tail", resolv_conf_d);
if (sl < 0 || (size_t)sl >= sizeof buf)
if (sl < 0 || (size_t)sl > sizeof buf)
log_line("snprintf failed appending resolv_conf_tail; path too long?");
else
resolv_conf_tail_fd = open(buf, O_RDONLY|O_CLOEXEC, 0);

View File

@ -23,12 +23,9 @@ static void get_leasefile_path(char *leasefile, size_t dlen, char *ifname)
{
int splen = snprintf(leasefile, dlen, "%s/LEASE-%s",
state_dir, ifname);
if (splen < 0)
if (splen < 0 || (size_t)splen > dlen)
suicide("%s: (%s) snprintf failed; return=%d",
client_config.interface, __func__, splen);
if ((size_t)splen >= dlen)
suicide("%s: (%s) snprintf dest buffer too small %d >= %zu",
client_config.interface, __func__, splen, sizeof dlen);
}
void open_leasefile(void)
@ -52,7 +49,7 @@ static void do_write_leasefile(struct in_addr ipnum)
}
inet_ntop(AF_INET, &ipnum, ip, sizeof ip);
ssize_t olen = snprintf(out, sizeof out, "%s\n", ip);
if (olen < 0 || (size_t)olen >= sizeof ip) {
if (olen < 0 || (size_t)olen > sizeof ip) {
log_line("%s: (%s) snprintf failed; return=%zd",
client_config.interface, __func__, olen);
return;

View File

@ -11,10 +11,8 @@ static inline void copy_cmdarg(char *dest, const char *src,
size_t destlen, const char *argname)
{
ssize_t olen = snprintf(dest, destlen, "%s", src);
if (olen < 0)
suicide("snprintf failed on %s; your system is broken?", argname);
if ((size_t)olen >= destlen)
suicide("snprintf would truncate %s arg; it's too long", argname);
if (olen < 0 || (size_t)olen > destlen)
suicide("snprintf failed on %s", argname);
}
#endif

View File

@ -143,7 +143,7 @@ static int create_udp_socket(uint32_t ip, uint16_t port, char *iface)
struct ifreq ifr;
memset(&ifr, 0, sizeof ifr);
ssize_t sl = snprintf(ifr.ifr_name, sizeof ifr.ifr_name, "%s", iface);
if (sl < 0 || (size_t)sl >= sizeof ifr.ifr_name) {
if (sl < 0 || (size_t)sl > sizeof ifr.ifr_name) {
log_line("%s: (%s) Set interface name failed.",
client_config.interface, __func__);
goto out_fd;