[src/trunk]: src/sys/external/bsd/ipf/netinet Fix use after free on packet wi...

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

details:   https://anonhg.NetBSD.org/src/rev/a464581d3ed2
branches:  trunk
changeset: 373334:a464581d3ed2
user:      christos <christos%NetBSD.org@localhost>
date:      Fri Feb 03 19:01:08 2023 +0000

description:
Fix use after free on packet with broken lengths

Under the scenario with a packet with length of 67 bytes, a header length
using the default of 20 bytes and a TCP data offset (th_off) of 48 will
cause m_pullup() to fail to make sure bytes are arranged contiguously.
m_pullup() will free the mbuf chain and return a null. ipfilter stores
the resultant mbuf address (or the resulting NULL) in its fr_info_t
structure. Unfortunately the erroneous packet is not flagged for drop.
>From FreeBSD via CY Schubert; originally reported by: Robert Morris
<rtm at lcs.mit.edu>

diffstat:

 sys/external/bsd/ipf/netinet/fil.c |  16 +++++++++++++---
 1 files changed, 13 insertions(+), 3 deletions(-)

diffs (52 lines):

diff -r 266ba25797fe -r a464581d3ed2 sys/external/bsd/ipf/netinet/fil.c
--- a/sys/external/bsd/ipf/netinet/fil.c        Fri Feb 03 15:59:04 2023 +0000
+++ b/sys/external/bsd/ipf/netinet/fil.c        Fri Feb 03 19:01:08 2023 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: fil.c,v 1.35 2021/12/05 07:28:20 msaitoh Exp $ */
+/*     $NetBSD: fil.c,v 1.36 2023/02/03 19:01:08 christos Exp $        */
 
 /*
  * Copyright (C) 2012 by Darren Reed.
@@ -141,7 +141,7 @@
 #if !defined(lint)
 #if defined(__NetBSD__)
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: fil.c,v 1.35 2021/12/05 07:28:20 msaitoh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: fil.c,v 1.36 2023/02/03 19:01:08 christos Exp $");
 #else
 static const char sccsid[] = "@(#)fil.c        1.36 6/5/96 (C) 1993-2000 Darren Reed";
 static const char rcsid[] = "@(#)Id: fil.c,v 1.1.1.2 2012/07/22 13:45:07 darrenr Exp $";
@@ -1142,8 +1142,10 @@
                if (M_LEN(fin->fin_m) < plen + fin->fin_ipoff) {
 #if defined(_KERNEL)
                        if (ipf_pullup(fin->fin_m, fin, plen) == NULL) {
-                               DT(ipf_pullup_fail);
+                               DT1(ipf_pullup_fail, fr_info_t *, fin);
                                LBUMP(ipf_stats[fin->fin_out].fr_pull[1]);
+                               fin->fin_reason = FRB_PULLUP;
+                               fin->fin_flx |= FI_BAD;
                                return -1;
                        }
                        LBUMP(ipf_stats[fin->fin_out].fr_pull[0]);
@@ -1156,6 +1158,7 @@
                        *fin->fin_mp = NULL;
                        fin->fin_m = NULL;
                        fin->fin_ip = NULL;
+                       fin->fin_flx |= FI_BAD;
                        return -1;
 #endif
                }
@@ -3214,6 +3217,13 @@
 
        SPL_X(s);
 
+       if (fin->fin_m == NULL && fin->fin_flx & FI_BAD &&
+           fin->fin_reason == FRB_PULLUP) {
+               /* m_pullup() has freed the mbuf */
+               LBUMP(ipf_stats[out].fr_blocked[fin->fin_reason]);
+               return (-1);
+       }
+
 #ifdef _KERNEL
        if (FR_ISPASS(pass))
                return 0;