[src/trunk]: src/sys/netipsec Fix the checks in compute_ipsec_pos, otherwise ...

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

details:   https://anonhg.NetBSD.org/src/rev/095d32f707ea
branches:  trunk
changeset: 361473:095d32f707ea
user:      maxv <maxv%NetBSD.org@localhost>
date:      Tue May 01 05:42:26 2018 +0000

description:
Fix the checks in compute_ipsec_pos, otherwise m_copydata could crash. I
already fixed half of the problem two months ago in rev1.67, back then I
thought it was not triggerable because each packet we emit is guaranteed
to have correctly formed IPv6 options; but it is actually triggerable via
IPv6 forwarding, we emit a packet we just received, and we don't sanitize
its options before invoking IPsec.

Since it would be wrong to just stop the iteration and continue the IPsec
processing, allow compute_ipsec_pos to fail, and when it does, drop the
packet entirely.

diffstat:

 sys/netipsec/ipsec_output.c |  30 +++++++++++++++++-------------
 1 files changed, 17 insertions(+), 13 deletions(-)

diffs (90 lines):

diff -r 9bf77aa46d15 -r 095d32f707ea sys/netipsec/ipsec_output.c
--- a/sys/netipsec/ipsec_output.c       Tue May 01 00:39:58 2018 +0000
+++ b/sys/netipsec/ipsec_output.c       Tue May 01 05:42:26 2018 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: ipsec_output.c,v 1.74 2018/04/28 15:45:16 maxv Exp $   */
+/*     $NetBSD: ipsec_output.c,v 1.75 2018/05/01 05:42:26 maxv Exp $   */
 
 /*
  * Copyright (c) 2002, 2003 Sam Leffler, Errno Consulting
@@ -29,7 +29,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: ipsec_output.c,v 1.74 2018/04/28 15:45:16 maxv Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ipsec_output.c,v 1.75 2018/05/01 05:42:26 maxv Exp $");
 
 #if defined(_KERNEL_OPT)
 #include "opt_inet.h"
@@ -623,7 +623,7 @@
 #endif
 
 #ifdef INET6
-static void
+static int
 compute_ipsec_pos(struct mbuf *m, int *i, int *off)
 {
        struct ip6_hdr *ip6 = mtod(m, struct ip6_hdr *);
@@ -640,7 +640,11 @@
         * put AH/ESP/IPcomp header.
         *     IPv6 hbh dest1 rthdr ah* [esp* dest2 payload]
         */
-       do {
+       while (1) {
+               if (*i + sizeof(ip6e) > m->m_pkthdr.len) {
+                       return EINVAL;
+               }
+
                switch (nxt) {
                case IPPROTO_AH:
                case IPPROTO_ESP:
@@ -649,7 +653,7 @@
                 * we should not skip security header added
                 * beforehand.
                 */
-                       return;
+                       return 0;
 
                case IPPROTO_HOPOPTS:
                case IPPROTO_DSTOPTS:
@@ -659,7 +663,7 @@
                 * we should stop there.
                 */
                        if (nxt == IPPROTO_DSTOPTS && dstopt)
-                               return;
+                               return 0;
 
                        if (nxt == IPPROTO_DSTOPTS) {
                                /*
@@ -679,16 +683,14 @@
                        m_copydata(m, *i, sizeof(ip6e), &ip6e);
                        nxt = ip6e.ip6e_nxt;
                        *off = *i + offsetof(struct ip6_ext, ip6e_nxt);
-                       /*
-                        * we will never see nxt == IPPROTO_AH
-                        * so it is safe to omit AH case.
-                        */
                        *i += (ip6e.ip6e_len + 1) << 3;
                        break;
                default:
-                       return;
+                       return 0;
                }
-       } while (*i + sizeof(ip6e) < m->m_pkthdr.len);
+       }
+
+       return 0;
 }
 
 static int
@@ -788,7 +790,9 @@
                i = ip->ip_hl << 2;
                off = offsetof(struct ip, ip_p);
        } else {
-               compute_ipsec_pos(m, &i, &off);
+               error = compute_ipsec_pos(m, &i, &off);
+               if (error)
+                       goto unrefsav;
        }
        error = (*sav->tdb_xform->xf_output)(m, isr, sav, NULL, i, off);
        KEY_SA_UNREF(&sav);