From 043bcc8d443424e47088f78053e96f95f77ee409 Mon Sep 17 00:00:00 2001 From: Brian Somers Date: Mon, 8 Jun 2009 21:42:30 +0000 Subject: [PATCH] Fix an off by one error when we limit append/prepend text sizes based on our internal buffer sizes. When we 'append', assume we're appending to text. Some MS dhcp servers will give us a string with the length including the trailing NUL. when we 'append domain-name', we get something like "search x.y\000 z" in resolv.conf :( MFC after: 1 week Security: A buffer overflow (by one NUL byte) was possible. --- sbin/dhclient/dhclient.c | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/sbin/dhclient/dhclient.c b/sbin/dhclient/dhclient.c index c23aba7f9852..f48466e9f45f 100644 --- a/sbin/dhclient/dhclient.c +++ b/sbin/dhclient/dhclient.c @@ -1977,7 +1977,7 @@ supersede: len = ip->client-> config->defaults[i].len + lease->options[i].len; - if (len > sizeof(dbuf)) { + if (len >= sizeof(dbuf)) { warning("no space to %s %s", "prepend option", dhcp_options[i].name); @@ -1996,24 +1996,34 @@ supersede: dp[len] = '\0'; break; case ACTION_APPEND: + /* + * When we append, we assume that we're + * appending to text. Some MS servers + * include a NUL byte at the end of + * the search string provided. + */ len = ip->client-> config->defaults[i].len + lease->options[i].len; - if (len > sizeof(dbuf)) { + if (len >= sizeof(dbuf)) { warning("no space to %s %s", "append option", dhcp_options[i].name); goto supersede; } - dp = dbuf; - memcpy(dp, + memcpy(dbuf, lease->options[i].data, lease->options[i].len); - memcpy(dp + lease->options[i].len, + for (dp = dbuf + lease->options[i].len; + dp > dbuf; dp--, len--) + if (dp[-1] != '\0') + break; + memcpy(dp, ip->client-> config->defaults[i].data, ip->client-> config->defaults[i].len); + dp = dbuf; dp[len] = '\0'; } } else {