Source-Changes-HG archive

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

[src/netbsd-7]: src/sys/netipsec Pull up following revision(s) (requested by ...



details:   https://anonhg.NetBSD.org/src/rev/5acf3d0eddb4
branches:  netbsd-7
changeset: 800415:5acf3d0eddb4
user:      martin <martin%NetBSD.org@localhost>
date:      Thu Feb 15 17:01:42 2018 +0000

description:
Pull up following revision(s) (requested by maxv in ticket #1569):
        sys/netipsec/xform_ah.c: revision 1.77, 1.81 (via patch)
        sys/netipsec/xform_esp.c: revision 1.73 (via patch)
        sys/netipsec/xform_ipip.c: revision 1.56, 1.57 (via patch)

Fix use-after-free. There is a path where the mbuf gets pulled up without
a proper mtod afterwards:

218     ipo = mtod(m, struct ip *);
281     m = m_pullup(m, hlen);
232     ipo->ip_src.s_addr

Found by Mootja.

Meanwhile it seems to me that 'ipo' should be set to NULL if the inner
packet is IPv6, but I'll revisit that later.

Reinforce and clarify.

Add missing NULL check. Normally that's not triggerable remotely, since we
are guaranteed that 8 bytes are valid at mbuf+skip.

As I said in my last commit in this file, ipo should be set to NULL;
otherwise the 'local address spoofing' check below is always wrong on
IPv6.

Make sure the Authentication Header fits the mbuf chain, otherwise panic.

diffstat:

 sys/netipsec/xform_ah.c   |  64 ++++++++++++++++++++--------------------------
 sys/netipsec/xform_esp.c  |   8 ++++-
 sys/netipsec/xform_ipip.c |   7 ++--
 3 files changed, 38 insertions(+), 41 deletions(-)

diffs (180 lines):

diff -r 74aff348ab77 -r 5acf3d0eddb4 sys/netipsec/xform_ah.c
--- a/sys/netipsec/xform_ah.c   Thu Feb 15 14:40:59 2018 +0000
+++ b/sys/netipsec/xform_ah.c   Thu Feb 15 17:01:42 2018 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: xform_ah.c,v 1.42.4.2 2018/02/15 08:03:08 martin Exp $ */
+/*     $NetBSD: xform_ah.c,v 1.42.4.3 2018/02/15 17:01:42 martin Exp $ */
 /*     $FreeBSD: src/sys/netipsec/xform_ah.c,v 1.1.4.1 2003/01/24 05:11:36 sam Exp $   */
 /*     $OpenBSD: ip_ah.c,v 1.63 2001/06/26 06:18:58 angelos Exp $ */
 /*
@@ -39,7 +39,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: xform_ah.c,v 1.42.4.2 2018/02/15 08:03:08 martin Exp $");
+__KERNEL_RCSID(0, "$NetBSD: xform_ah.c,v 1.42.4.3 2018/02/15 17:01:42 martin Exp $");
 
 #include "opt_inet.h"
 #ifdef __FreeBSD__
@@ -498,54 +498,45 @@
 
                nxt = ip6.ip6_nxt & 0xff; /* Next header type. */
 
-               for (off = 0; off < skip - sizeof(struct ip6_hdr);)
+               for (off = 0; off < skip - sizeof(struct ip6_hdr);) {
+                       int noff;
+
                        switch (nxt) {
                        case IPPROTO_HOPOPTS:
                        case IPPROTO_DSTOPTS:
-                               ip6e = (struct ip6_ext *) (ptr + off);
+                               ip6e = (struct ip6_ext *)(ptr + off);
+                               noff = off + ((ip6e->ip6e_len + 1) << 3);
+
+                               /* Sanity check. */
+                               if (noff > skip - sizeof(struct ip6_hdr)) {
+                                       goto error6;
+                               }
 
                                /*
-                                * Process the mutable/immutable
-                                * options -- borrows heavily from the
-                                * KAME code.
+                                * Zero out mutable options.
                                 */
                                for (count = off + sizeof(struct ip6_ext);
-                                    count < off + ((ip6e->ip6e_len + 1) << 3);) {
+                                    count < noff;) {
                                        if (ptr[count] == IP6OPT_PAD1) {
                                                count++;
-                                               continue; /* Skip padding. */
-                                       }
-
-                                       /* Sanity check. */
-                                       if (count > off +
-                                           ((ip6e->ip6e_len + 1) << 3)) {
-                                               m_freem(m);
-
-                                               /* Free, if we allocated. */
-                                               if (alloc)
-                                                       free(ptr, M_XDATA);
-                                               return EINVAL;
+                                               continue;
                                        }
 
                                        ad = ptr[count + 1] + 2;
 
-                                       /* If mutable option, zeroize. */
-                                       if (ptr[count] & IP6OPT_MUTABLE)
-                                               memcpy(ptr + count, ipseczeroes,
-                                                   ad);
+                                       if (count + ad > noff) {
+                                               goto error6;
+                                       }
+
+                                       if (ptr[count] & IP6OPT_MUTABLE) {
+                                               memset(ptr + count, 0, ad);
+                                       }
 
                                        count += ad;
-
-                                       /* Sanity check. */
-                                       if (count >
-                                           skip - sizeof(struct ip6_hdr)) {
-                                               m_freem(m);
+                               }
 
-                                               /* Free, if we allocated. */
-                                               if (alloc)
-                                                       free(ptr, M_XDATA);
-                                               return EINVAL;
-                                       }
+                               if (count != noff) {
+                                       goto error6;
                                }
 
                                /* Advance. */
@@ -603,11 +594,13 @@
                        default:
                                DPRINTF(("ah_massage_headers: unexpected "
                                    "IPv6 header type %d", off));
+error6:
                                if (alloc)
                                        free(ptr, M_XDATA);
                                m_freem(m);
                                return EINVAL;
                        }
+               }
 
                /* Copyback and free, if we allocated. */
                if (alloc) {
@@ -687,11 +680,10 @@
                return EACCES;
        }
        if (skip + authsize + rplen > m->m_pkthdr.len) {
-               char buf[IPSEC_ADDRSTRLEN];
                DPRINTF(("%s: bad mbuf length %u (expecting >= %lu)"
                        " for packet in SA %s/%08lx\n", __func__,
                        m->m_pkthdr.len, (u_long)(skip + authsize + rplen),
-                       ipsec_address(&sav->sah->saidx.dst, buf, sizeof(buf)),
+                       ipsec_address(&sav->sah->saidx.dst),
                        (u_long) ntohl(sav->spi)));
                AH_STATINC(AH_STAT_BADAUTHL);
                m_freem(m);
diff -r 74aff348ab77 -r 5acf3d0eddb4 sys/netipsec/xform_esp.c
--- a/sys/netipsec/xform_esp.c  Thu Feb 15 14:40:59 2018 +0000
+++ b/sys/netipsec/xform_esp.c  Thu Feb 15 17:01:42 2018 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: xform_esp.c,v 1.45 2013/11/03 18:37:10 mrg Exp $       */
+/*     $NetBSD: xform_esp.c,v 1.45.4.1 2018/02/15 17:01:42 martin Exp $        */
 /*     $FreeBSD: src/sys/netipsec/xform_esp.c,v 1.2.2.1 2003/01/24 05:11:36 sam Exp $  */
 /*     $OpenBSD: ip_esp.c,v 1.69 2001/06/26 06:18:59 angelos Exp $ */
 
@@ -39,7 +39,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: xform_esp.c,v 1.45 2013/11/03 18:37:10 mrg Exp $");
+__KERNEL_RCSID(0, "$NetBSD: xform_esp.c,v 1.45.4.1 2018/02/15 17:01:42 martin Exp $");
 
 #include "opt_inet.h"
 #ifdef __FreeBSD__
@@ -315,6 +315,10 @@
 
        /* XXX don't pullup, just copy header */
        IP6_EXTHDR_GET(esp, struct newesp *, m, skip, sizeof (struct newesp));
+       if (esp == NULL) {
+               /* m already freed */
+               return EINVAL;
+       }
 
        esph = sav->tdb_authalgxform;
        espx = sav->tdb_encalgxform;
diff -r 74aff348ab77 -r 5acf3d0eddb4 sys/netipsec/xform_ipip.c
--- a/sys/netipsec/xform_ipip.c Thu Feb 15 14:40:59 2018 +0000
+++ b/sys/netipsec/xform_ipip.c Thu Feb 15 17:01:42 2018 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: xform_ipip.c,v 1.31.2.1 2018/02/15 14:39:43 martin Exp $       */
+/*     $NetBSD: xform_ipip.c,v 1.31.2.2 2018/02/15 17:01:42 martin Exp $       */
 /*     $FreeBSD: src/sys/netipsec/xform_ipip.c,v 1.3.2.1 2003/01/24 05:11:36 sam Exp $ */
 /*     $OpenBSD: ip_ipip.c,v 1.25 2002/06/10 18:04:55 itojun Exp $ */
 
@@ -39,7 +39,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: xform_ipip.c,v 1.31.2.1 2018/02/15 14:39:43 martin Exp $");
+__KERNEL_RCSID(0, "$NetBSD: xform_ipip.c,v 1.31.2.2 2018/02/15 17:01:42 martin Exp $");
 
 /*
  * IP-inside-IP processing
@@ -323,7 +323,8 @@
 #endif /* INET */
 #ifdef INET6
        case 6:
-                ip6 = (struct ip6_hdr *) ipo;
+               ipo = NULL;
+               ip6 = mtod(m, struct ip6_hdr *);
                itos = (ntohl(ip6->ip6_flow) >> 20) & 0xff;
                ip_ecn_egress(ip6_ipsec_ecn, &otos, &itos);
                ip6->ip6_flow &= ~htonl(0xff << 20);



Home | Main Index | Thread Index | Old Index