Source-Changes-HG archive

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

[src/netbsd-6-1]: src/sys/netipsec Pull up following revision(s) (requested b...



details:   https://anonhg.NetBSD.org/src/rev/edc3f872e701
branches:  netbsd-6-1
changeset: 776309:edc3f872e701
user:      snj <snj%NetBSD.org@localhost>
date:      Tue Mar 13 17:18:14 2018 +0000

description:
Pull up following revision(s) (requested by maxv in ticket #1532):
        sys/netipsec/xform_ah.c: 1.77 via patch
        sys/netipsec/xform_esp.c: 1.73 via patch
        sys/netipsec/xform_ipip.c: 1.56-1.57 via patch
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.
--
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.
--
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.

diffstat:

 sys/netipsec/xform_ah.c   |  61 ++++++++++++++++++++--------------------------
 sys/netipsec/xform_esp.c  |   8 ++++-
 sys/netipsec/xform_ipip.c |   7 +++--
 3 files changed, 37 insertions(+), 39 deletions(-)

diffs (167 lines):

diff -r 338b03ed7174 -r edc3f872e701 sys/netipsec/xform_ah.c
--- a/sys/netipsec/xform_ah.c   Tue Mar 13 17:01:55 2018 +0000
+++ b/sys/netipsec/xform_ah.c   Tue Mar 13 17:18:14 2018 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: xform_ah.c,v 1.37.8.3 2018/02/15 16:49:35 martin Exp $ */
+/*     $NetBSD: xform_ah.c,v 1.37.8.4 2018/03/13 17:18:14 snj 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.37.8.3 2018/02/15 16:49:35 martin Exp $");
+__KERNEL_RCSID(0, "$NetBSD: xform_ah.c,v 1.37.8.4 2018/03/13 17:18:14 snj 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) {
diff -r 338b03ed7174 -r edc3f872e701 sys/netipsec/xform_esp.c
--- a/sys/netipsec/xform_esp.c  Tue Mar 13 17:01:55 2018 +0000
+++ b/sys/netipsec/xform_esp.c  Tue Mar 13 17:18:14 2018 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: xform_esp.c,v 1.40 2012/01/25 20:31:23 drochner Exp $  */
+/*     $NetBSD: xform_esp.c,v 1.40.8.1 2018/03/13 17:18:14 snj 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.40 2012/01/25 20:31:23 drochner Exp $");
+__KERNEL_RCSID(0, "$NetBSD: xform_esp.c,v 1.40.8.1 2018/03/13 17:18:14 snj 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 338b03ed7174 -r edc3f872e701 sys/netipsec/xform_ipip.c
--- a/sys/netipsec/xform_ipip.c Tue Mar 13 17:01:55 2018 +0000
+++ b/sys/netipsec/xform_ipip.c Tue Mar 13 17:18:14 2018 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: xform_ipip.c,v 1.28.22.1 2018/02/15 14:50:17 martin Exp $      */
+/*     $NetBSD: xform_ipip.c,v 1.28.22.2 2018/03/13 17:18:14 snj 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.28.22.1 2018/02/15 14:50:17 martin Exp $");
+__KERNEL_RCSID(0, "$NetBSD: xform_ipip.c,v 1.28.22.2 2018/03/13 17:18:14 snj Exp $");
 
 /*
  * IP-inside-IP processing
@@ -324,7 +324,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