[src/trunk]: src/dist/dhcp Fix CVS-2011-2748, malformed packets can cause ser...

[christos <christos%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src/rev/aff92b4da499
branches:  trunk
changeset: 768376:aff92b4da499
user:      christos <christos%NetBSD.org@localhost>
date:      Mon Aug 15 21:12:43 2011 +0000

description:
Fix CVS-2011-2748, malformed packets can cause server to crash causing a DoS.

diffstat:

 dist/dhcp/common/discover.c |  13 ++++++++++++-
 dist/dhcp/common/options.c  |  31 +++++++++++++++++++------------
 dist/dhcp/server/dhcp.c     |   6 +++++-
 3 files changed, 36 insertions(+), 14 deletions(-)

diffs (124 lines):

diff -r 69274b0c3194 -r aff92b4da499 dist/dhcp/common/discover.c
--- a/dist/dhcp/common/discover.c       Mon Aug 15 21:00:49 2011 +0000
+++ b/dist/dhcp/common/discover.c       Mon Aug 15 21:12:43 2011 +0000
@@ -34,7 +34,7 @@
 
 #ifndef lint
 static char copyright[] =
-"$Id: discover.c,v 1.11 2007/10/31 15:26:51 gdt Exp $ Copyright (c) 2004-2005 Internet Systems Consortium.  All rights reserved.\n";
+"$Id: discover.c,v 1.12 2011/08/15 21:12:43 christos Exp $ Copyright (c) 2004-2005 Internet Systems Consortium.  All rights reserved.\n";
 #endif /* not lint */
 
 #include "dhcpd.h"
@@ -830,6 +830,17 @@
        }
        if (result == 0)
                return ISC_R_UNEXPECTED;
+       /*
+        * If we didn't at least get the fixed portion of the BOOTP
+        * packet, drop the packet.
+        * Previously we allowed packets with no sname or filename
+        * as we were aware of at least one client that did.  But
+        * a bug caused short packets to not work and nobody has
+        * complained, it seems rational to tighten up that
+        * restriction.
+        */
+       if (result < DHCP_FIXED_NON_UDP)
+               return ISC_R_UNEXPECTED;
 
        if (bootp_packet_handler) {
                ifrom.len = 4;
diff -r 69274b0c3194 -r aff92b4da499 dist/dhcp/common/options.c
--- a/dist/dhcp/common/options.c        Mon Aug 15 21:00:49 2011 +0000
+++ b/dist/dhcp/common/options.c        Mon Aug 15 21:12:43 2011 +0000
@@ -34,7 +34,7 @@
 
 #ifndef lint
 static char copyright[] =
-"$Id: options.c,v 1.6 2006/05/11 09:29:39 mrg Exp $ Copyright (c) 2004 Internet Systems Consortium.  All rights reserved.\n";
+"$Id: options.c,v 1.7 2011/08/15 21:12:43 christos Exp $ Copyright (c) 2004 Internet Systems Consortium.  All rights reserved.\n";
 #endif /* not lint */
 
 #define DHCP_OPTION_DATA
@@ -499,19 +499,26 @@
           honor it. */
 
        if (mms) {
-               main_buffer_size = mms - DHCP_FIXED_LEN;
+               if (mms < 576)
+                       /* Enforce a minimum packet size... */
+                       main_buffer_size = mms - DHCP_FIXED_LEN;
+               else if (mms > 1500)
+                       /*
+                        * TODO: Packets longer than 1500 bytes really
+                        * should be allowed, but it requires upstream
+                        * changes to the way the packet is allocated.  For
+                        * now, we forbid them.  They won't be needed very
+                        * often anyway.
+                        */
+                       main_buffer_size = 1500 - DHCP_FIXED_LEN;
+               else
+                       main_buffer_size = mms - DHCP_FIXED_LEN;
 
-               /* Enforce a minimum packet size... */
-               if (main_buffer_size < (576 - DHCP_FIXED_LEN))
-                       main_buffer_size = 576 - DHCP_FIXED_LEN;
        } else if (bootpp) {
-               if (inpacket) {
-                       main_buffer_size =
-                               inpacket -> packet_length - DHCP_FIXED_LEN;
-                       if (main_buffer_size < 64)
-                               main_buffer_size = 64;
-               } else
-                       main_buffer_size = 64;
+               main_buffer_size = 64;
+               if (inpacket != NULL &&
+                   (inpacket->packet_length >= 64 + DHCP_FIXED_NON_UDP))
+                       main_buffer_size = inpacket->packet_length - DHCP_FIXED_NON_UDP;
        } else
                main_buffer_size = 576 - DHCP_FIXED_LEN;
 
diff -r 69274b0c3194 -r aff92b4da499 dist/dhcp/server/dhcp.c
--- a/dist/dhcp/server/dhcp.c   Mon Aug 15 21:00:49 2011 +0000
+++ b/dist/dhcp/server/dhcp.c   Mon Aug 15 21:12:43 2011 +0000
@@ -34,7 +34,7 @@
 
 #ifndef lint
 static char copyright[] =
-"$Id: dhcp.c,v 1.11 2009/07/16 22:44:27 tonnerre Exp $ Copyright (c) 2004-2005 Internet Systems Consortium.  All rights reserved.\n";
+"$Id: dhcp.c,v 1.12 2011/08/15 21:12:43 christos Exp $ Copyright (c) 2004-2005 Internet Systems Consortium.  All rights reserved.\n";
 #endif /* not lint */
 
 #include "dhcpd.h"
@@ -128,6 +128,7 @@
        if (packet -> packet_type == DHCPREQUEST &&
            packet -> raw -> ciaddr.s_addr &&
            !packet -> raw -> giaddr.s_addr &&
+           packet -> options != NULL &&
            (packet -> options -> universe_count < agent_universe.index ||
             !packet -> options -> universes [agent_universe.index]))
        {
@@ -1379,6 +1380,7 @@
        /* If there were agent options in the incoming packet, return
           them. */
        if (packet -> raw -> giaddr.s_addr &&
+           packet -> options != NULL &&
            packet -> options -> universe_count > agent_universe.index &&
            packet -> options -> universes [agent_universe.index]) {
                option_chain_head_reference
@@ -1536,6 +1538,7 @@
           them.  Do not return the agent options if they were stashed
           on the lease. */
        if (packet -> raw -> giaddr.s_addr &&
+           packet -> options != NULL &&
            packet -> options -> universe_count > agent_universe.index &&
            packet -> options -> universes [agent_universe.index] &&
            (state -> options -> universe_count <= agent_universe.index ||
@@ -2179,6 +2182,7 @@
           in with the packet, so that we can use them at renewal time when
           the packet won't have gone through a relay agent. */
        if (packet -> raw -> giaddr.s_addr &&
+           packet -> options != NULL &&
            packet -> options -> universe_count > agent_universe.index &&
            packet -> options -> universes [agent_universe.index] &&
            (state -> options -> universe_count <= agent_universe.index ||