Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[src/trunk]: src/sys/netinet Fix a buffer overflow in icmp_error. We create i...



details:   https://anonhg.NetBSD.org/src/rev/6d39539d5e91
branches:  trunk
changeset: 829149:6d39539d5e91
user:      maxv <maxv%NetBSD.org@localhost>
date:      Fri Jan 19 13:17:29 2018 +0000

description:
Fix a buffer overflow in icmp_error. We create in 'm' a packet that must
contain:

  IPv4 header | Fixed part of ICMP header | Variable part of ICMP header

But we perform length checks on 'totlen', which does not count the IPv4
header.

So now, add sizeof(struct ip) in totlen, and stop doing this m_data
nonsense, just get the pointers as usual.

diffstat:

 sys/netinet/ip_icmp.c |  48 ++++++++++++++++++------------------------------
 1 files changed, 18 insertions(+), 30 deletions(-)

diffs (102 lines):

diff -r b2c39426f3f6 -r 6d39539d5e91 sys/netinet/ip_icmp.c
--- a/sys/netinet/ip_icmp.c     Fri Jan 19 12:50:27 2018 +0000
+++ b/sys/netinet/ip_icmp.c     Fri Jan 19 13:17:29 2018 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: ip_icmp.c,v 1.162 2018/01/19 12:50:27 maxv Exp $       */
+/*     $NetBSD: ip_icmp.c,v 1.163 2018/01/19 13:17:29 maxv Exp $       */
 
 /*
  * Copyright (C) 1995, 1996, 1997, and 1998 WIDE Project.
@@ -94,7 +94,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: ip_icmp.c,v 1.162 2018/01/19 12:50:27 maxv Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ip_icmp.c,v 1.163 2018/01/19 13:17:29 maxv Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_ipsec.h"
@@ -260,7 +260,8 @@
        struct icmp *icp;
        struct mbuf *m;
        struct m_tag *mtag;
-       unsigned datalen, mblen, totlen;
+       unsigned datalen, mblen;
+       int totlen;
 
 #ifdef ICMPPRINTFS
        if (icmpprintfs)
@@ -315,10 +316,12 @@
         * Compute the total length of the new packet. Truncate it if it's
         * bigger than the size of a cluster.
         */
-       CTASSERT(ICMP_MINLEN <= MCLBYTES);
-       if (datalen + ICMP_MINLEN > MCLBYTES)
-               datalen = MCLBYTES - ICMP_MINLEN;
-       totlen = datalen + ICMP_MINLEN;
+       CTASSERT(ICMP_MINLEN + sizeof(struct ip) <= MCLBYTES);
+       totlen = sizeof(struct ip) + ICMP_MINLEN + datalen;
+       if (totlen > MCLBYTES) {
+               datalen = MCLBYTES - ICMP_MINLEN - sizeof(struct ip);
+               totlen = MCLBYTES;
+       }
 
        /*
         * Allocate the mbuf for the new packet.
@@ -335,26 +338,23 @@
                goto freeit;
        MCLAIM(m, n->m_owner);
        m->m_len = totlen;
-
-       /*
-        * Advance to the ICMP header.
-        */
-       if ((m->m_flags & M_EXT) == 0) {
-               MH_ALIGN(m, m->m_len);
-       } else {
-               m->m_data += sizeof(struct ip);
-               m->m_len -= sizeof(struct ip);
-       }
+       m->m_pkthdr.len = m->m_len;
+       m_copy_rcvif(m, n);
 
        if ((u_int)type > ICMP_MAXTYPE)
                panic("icmp_error");
        ICMP_STATINC(ICMP_STAT_OUTHIST + type);
 
        /*
+        * Get pointers on the IP header and the ICMP header.
+        */
+       nip = mtod(m, struct ip *);
+       icp = (struct icmp *)(nip + 1);
+
+       /*
         * Fill in the fields of the ICMP header: icmp_type, icmp_code
         * and icmp_ip. icmp_cksum gets filled later.
         */
-       icp = mtod(m, struct icmp *);
        icp->icmp_type = type;
        if (type == ICMP_REDIRECT) {
                icp->icmp_gwaddr.s_addr = dest;
@@ -375,21 +375,9 @@
        m_copydata(n, 0, datalen, (void *)&icp->icmp_ip);
 
        /*
-        * Come back to the IP header.
-        */
-       if ((m->m_flags & M_EXT) == 0 &&
-           m->m_data - sizeof(struct ip) < m->m_pktdat)
-               panic("icmp len");
-       m->m_data -= sizeof(struct ip);
-       m->m_len += sizeof(struct ip);
-       m->m_pkthdr.len = m->m_len;
-       m_copy_rcvif(m, n);
-
-       /*
         * Now, copy the old IP header (without options) in front of the
         * ICMP message. The src/dst fields will be swapped in icmp_reflect.
         */
-       nip = mtod(m, struct ip *);
        /* ip_v set in ip_output */
        nip->ip_hl = sizeof(struct ip) >> 2;
        nip->ip_tos = 0;



Home | Main Index | Thread Index | Old Index