Mercurial > hg > dhcpcd
changeset 70:c82dda9e5b7b draft
Don't crash with 0 or invalid length DHCP options, reported by Stefan de Konink.
| author | Roy Marples <roy@marples.name> |
|---|---|
| date | Tue, 27 Feb 2007 12:06:44 +0000 |
| parents | 03eaa094143b |
| children | 10f4e33d41f5 |
| files | ChangeLog client.c dhcp.c dhcpcd.8 socket.c |
| diffstat | 5 files changed, 82 insertions(+), 18 deletions(-) [+] |
line wrap: on
line diff
--- a/ChangeLog Fri Feb 23 08:44:38 2007 +0000 +++ b/ChangeLog Tue Feb 27 12:06:44 2007 +0000 @@ -1,3 +1,6 @@ +Don't crash with 0 or invalid length DHCP options, reported by +Stefan de Konink. + dhcpcd-3.0.13 Fix regression on Linux for sending packets over non Ethernet devices. define ARPHRD_IEEE1394 if it doesn not exist, like for Linux-2.4 kernels.
--- a/client.c Fri Feb 23 08:44:38 2007 +0000 +++ b/client.c Tue Feb 27 12:06:44 2007 +0000 @@ -533,7 +533,7 @@ { if (! dhcp->leasetime) { - dhcp->leasetime = DEFAULT_TIMEOUT; + dhcp->leasetime = DEFAULT_LEASETIME; logger(LOG_INFO, "no lease time supplied, assuming %d seconds", dhcp->leasetime);
--- a/dhcp.c Fri Feb 23 08:44:38 2007 +0000 +++ b/dhcp.c Tue Feb 27 12:06:44 2007 +0000 @@ -456,21 +456,36 @@ } } -static void dhcp_add_address(address_t *address, const unsigned char *data, int length) +static bool dhcp_add_address(address_t **address, const unsigned char *data, int length) { int i; - address_t *p = address; + address_t *p = *address; for (i = 0; i < length; i += 4) { - memset (p, 0, sizeof (address_t)); - memcpy (&p->address.s_addr, data + i, 4); - if (length - i > 4) + if (*address == NULL) + { + *address = xmalloc (sizeof (address_t)); + p = *address; + } + else { p->next = xmalloc (sizeof (address_t)); p = p->next; } + memset (p, 0, sizeof (address_t)); + + /* Sanity check */ + if (i + 4 > length) + { + logger (LOG_ERR, "invalid address length"); + return (false); + } + + memcpy (&p->address.s_addr, data + i, 4); } + + return (true); } int parse_dhcpmessage (dhcp_t *dhcp, const dhcpmessage_t *message) @@ -494,6 +509,13 @@ dhcp->address.s_addr = message->yiaddr; strcpy (dhcp->servername, message->servername); +#define LEN_ERR \ + { \ + logger (LOG_ERR, "invalid length %d for option %d", length, option); \ + retval = -1; \ + goto eexit; \ + } + while (p < end) { option = *p++; @@ -504,6 +526,7 @@ if (p + length >= end) { + logger (LOG_ERR, "dhcp option exceeds message length"); retval = -1; goto eexit; } @@ -515,12 +538,33 @@ case DHCP_MESSAGETYPE: retval = (int) *p; - break; + p += length; + continue; + + default: + if (length == 0) + { + logger (LOG_DEBUG, "option %d has zero length, skipping", + option); + continue; + } + } + +#define MIN_LENGTH(_length) \ + if (length < _length) \ + LEN_ERR; +#define MULT_LENGTH(_mult) \ + if (length % _mult != 0) \ + LEN_ERR; #define GET_UINT32(_val) \ - memcpy (&_val, p, sizeof (uint32_t)); + MIN_LENGTH (sizeof (uint32_t)); \ + memcpy (&_val, p, sizeof (uint32_t)); #define GET_UINT32_H(_val) \ - GET_UINT32 (_val); \ - _val = ntohl (_val); + GET_UINT32 (_val); \ + _val = ntohl (_val); + + switch (option) + { case DHCP_ADDRESS: GET_UINT32 (dhcp->address.s_addr); break; @@ -552,6 +596,7 @@ #undef GET_UINT32 #define GETSTR(_var) \ + MIN_LENGTH (sizeof (char)); \ if (_var) free (_var); \ _var = xmalloc (length + 1); \ memcpy (_var, p, length); \ @@ -574,9 +619,12 @@ #undef GETSTR #define GETADDR(_var) \ - if (_var) free (_var); \ - _var = xmalloc (sizeof (address_t)); \ - dhcp_add_address (_var, p, length); + MULT_LENGTH (4); \ + if (! dhcp_add_address (&_var, p, length)) \ + { \ + retval = -1; \ + goto eexit; \ + } case DHCP_DNSSERVER: GETADDR (dhcp->dnsservers); break; @@ -589,9 +637,10 @@ #undef GETADDR case DHCP_DNSSEARCH: + MIN_LENGTH (1); if (dhcp->dnssearch) free (dhcp->dnssearch); - if ((len = decode_search (p, length, NULL))) + if ((len = decode_search (p, length, NULL)) > 0) { dhcp->dnssearch = xmalloc (len); decode_search (p, length, dhcp->dnssearch); @@ -599,10 +648,14 @@ break; case DHCP_CSR: + MIN_LENGTH (5); + if (csr) + free_route (csr); csr = decodeCSR (p, length); break; case DHCP_STATICROUTE: + MULT_LENGTH (8); for (i = 0; i < length; i += 8) { memcpy (&route->destination.s_addr, p + i, 4); @@ -616,6 +669,7 @@ break; case DHCP_ROUTERS: + MULT_LENGTH (4); for (i = 0; i < length; i += 4) { memcpy (&route->gateway.s_addr, p + i, 4); @@ -626,6 +680,9 @@ } break; +#undef MIN_LENGTH +#undef MULT_LENGTH + default: logger (LOG_DEBUG, "no facility to parse DHCP code %u", option); break;
--- a/dhcpcd.8 Fri Feb 23 08:44:38 2007 +0000 +++ b/dhcpcd.8 Tue Feb 27 12:06:44 2007 +0000 @@ -1,6 +1,6 @@ .\" $Id$ .\" -.TH DHCPCD 8 "12 February 2007" "dhcpcd 3.0" +.TH DHCPCD 8 "27 February 2007" "dhcpcd 3.0" .SH NAME dhcpcd \- DHCP client daemon @@ -110,7 +110,9 @@ used in the .B DHCP_DISCOVER message. Use -1 for an infinite lease time. We don't request a specific -lease time by default. +lease time by default. If we do not receive a lease time in the +.B DHCP_OFFER +message then we default to 1 hour. .TP .BI \-m \ metric Routes will be added with the given metric. The default is 0. @@ -142,7 +144,9 @@ lease period when the lease is not renewed. .TP .BI \-s \ ipaddr -Sends DHCP_REQUEST message requesting to lease IP address ipaddr. +Sends +.B DHCP_REQUEST +message requesting to lease IP address ipaddr. The ipaddr parameter must be in the form xxx.xxx.xxx.xxx. This effectively doubles the timeout period, as if we fail to get this IP address then we enter the INIT state and try to get any @@ -322,7 +326,6 @@ draft-ietf-dhc-fqdn-option .SH BUGS -Probably many. Please report them to http://bugs.gentoo.org. .PD 0