From: Roy Marples <roy@marples.name>
Date: Tue, 27 Feb 2007 12:06:44 +0000 (+0000)
Subject: Don't crash with 0 or invalid length DHCP options, reported by Stefan de Konink.
X-Git-Tag: v3.2.3~304
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=9e3dfd633db5bab1f188af064ee937575fb2171c;p=thirdparty%2Fdhcpcd.git

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

diff --git a/ChangeLog b/ChangeLog
index 793a33c9..92c0351c 100644
--- 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 c3a1bf66..08373dfd 100644
--- 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 90a0e012..3af0672c 100644
--- 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 b7fe0d79..0f0c4543 100644
--- 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 a1726b4a..2ad97906 100644
--- 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;