Don't crash with 0 or invalid length DHCP options, reported by Stefan de Konink.

diff --git a/ChangeLog b/ChangeLog
index 793a33c9ea47f915a1f6f5550b296db4d6d2ce00..92c0351ce5815f58042b22b1562dc68caf24a9b8 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -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.

diff --git a/client.c b/client.c
index c3a1bf663140c2ed991868534c6fea8e5834056a..08373dfd5feac1aa15a8c1c42c4f0acdf6a3e50b 100644 (file)
--- a/client.c
+++ b/client.c
@@ -533,7 +533,7 @@ int dhcp_run (const options_t *options)
                    {
                      if (! dhcp->leasetime)
                        {
-                         dhcp->leasetime = DEFAULT_TIMEOUT;
+                         dhcp->leasetime = DEFAULT_LEASETIME;
                          logger(LOG_INFO,
                                 "no lease time supplied, assuming %d seconds",
                                 dhcp->leasetime);

diff --git a/dhcp.c b/dhcp.c
index 90a0e012dc44a69370eb0c9ef26505dfbe565ea5..3af0672c7d771da9bfb1a3069edf566f96821b47 100644 (file)
--- a/dhcp.c
+++ b/dhcp.c
@@ -456,21 +456,36 @@ void free_dhcp (dhcp_t *dhcp)
     }
 }
 
-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 @@ int parse_dhcpmessage (dhcp_t *dhcp, const dhcpmessage_t *message)
   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 @@ int parse_dhcpmessage (dhcp_t *dhcp, const dhcpmessage_t *message)
 
       if (p + length >= end)
        {
+         logger (LOG_ERR, "dhcp option exceeds message length");
          retval = -1;
          goto eexit;
        }
@@ -515,12 +538,33 @@ int parse_dhcpmessage (dhcp_t *dhcp, const dhcpmessage_t *message)
 
        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 @@ int parse_dhcpmessage (dhcp_t *dhcp, const dhcpmessage_t *message)
 #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 @@ int parse_dhcpmessage (dhcp_t *dhcp, const dhcpmessage_t *message)
 #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 @@ int parse_dhcpmessage (dhcp_t *dhcp, const dhcpmessage_t *message)
 #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 @@ int parse_dhcpmessage (dhcp_t *dhcp, const dhcpmessage_t *message)
          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 @@ int parse_dhcpmessage (dhcp_t *dhcp, const dhcpmessage_t *message)
          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 @@ int parse_dhcpmessage (dhcp_t *dhcp, const dhcpmessage_t *message)
            }
          break;
 
+#undef MIN_LENGTH
+#undef MULT_LENGTH
+
        default:
          logger (LOG_DEBUG, "no facility to parse DHCP code %u", option);
          break;

diff --git a/dhcpcd.8 b/dhcpcd.8
index b7fe0d79978efbe798f52431bca0b0ab0504bac5..0f0c4543fd52474cedc501d280c3f90a74f0dfad 100644 (file)
--- a/dhcpcd.8
+++ b/dhcpcd.8
@@ -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 @@ that the server can override this value if it sees fit). This value is
 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 @@ is stopped it can no longer down an interface at the end of its
 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 @@ RFC2132
 draft-ietf-dhc-fqdn-option
 
 .SH BUGS
-Probably many.
 Please report them to http://bugs.gentoo.org.
 .PD 0
 

diff --git a/socket.c b/socket.c
index a1726b4a70304b095fa13a990d701526f30f64ab..2ad97906a6364cdcd9e03333a83e6cb590ce991e 100644 (file)
--- a/socket.c
+++ b/socket.c
@@ -522,6 +522,7 @@ int get_packet (const interface_t *iface, unsigned char *data,
 
   memset (buffer, 0, iface->buffer_length);
   bytes = read (iface->fd, buffer, iface->buffer_length);
+
   if (bytes < 0)
     {
       struct timeval tv;