Bug 288333 - ipfilter: NULL dereference in ipf_pr_icmp6
Summary: ipfilter: NULL dereference in ipf_pr_icmp6
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: 15.0-CURRENT
Hardware: Any Any
: --- Affects Some People
Assignee: Cy Schubert
URL:
Keywords: crash
Depends on:
Blocks:
 
Reported: 2025-07-19 19:21 UTC by Robert Morris
Modified: 2026-05-27 13:45 UTC (History)
3 users (show)

See Also:


Attachments
send a packet that causes ipf_pr_icmp6() to dereference NULL (4.46 KB, text/plain)
2025-07-19 19:21 UTC, Robert Morris
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Robert Morris 2025-07-19 19:21:04 UTC
Created attachment 262288 [details]
send a packet that causes ipf_pr_icmp6() to dereference NULL

I've attached a demo program that sets up ipf with

  pass out quick on tap1 proto tcp from any to any port = 22 flags S keep state

and sends an outgoing tap0->tap1 inet6/tcp packet to set up state, and
then sends an inward inet6/IPPROTO_AH packet on tap1 that, because it
has ttl=0, is rejected by ip6_forward(). As part of generating an
ICMPV6 error packet, ipf_checkicmp6matchingstate() says

        ofin.fin_m = NULL;      /* if dereferenced, panic XXX */
        ...
        (void) ipf_makefrip(sizeof(*oip6), (ip_t *)oip6, &ofin);

the latter causes ipf_pr_icmp6() to be called, which says

                case ICMP6_DST_UNREACH :
                case ICMP6_PACKET_TOO_BIG :
                case ICMP6_TIME_EXCEEDED :
                case ICMP6_PARAM_PROB :
                    ...
                        if (M_LEN(fin->fin_m) < fin->fin_plen) {

#0  0xffffffc0000885d2 in ipf_pr_icmp6 (fin=0xffffffc082adfd90)
    at /usr/rtm/symbsd/src/sys/netpfil/ipfilter/netinet/fil.c:908
#1  ipf_pr_ipv6hdr (fin=0xffffffc082adfd90)
    at /usr/rtm/symbsd/src/sys/netpfil/ipfilter/netinet/fil.c:481
#2  ipf_makefrip (hlen=<optimized out>, ip=<optimized out>, 
    fin=0xffffffc082adfd90)
    at /usr/rtm/symbsd/src/sys/netpfil/ipfilter/netinet/fil.c:2023
#3  0xffffffc0000b1972 in ipf_checkicmp6matchingstate (fin=<optimized out>)
    at /usr/rtm/symbsd/src/sys/netpfil/ipfilter/netinet/ip_state.c:4391
#4  0xffffffc0000b0f62 in ipf_state_lookup (fin=0xffffffc082ae00d0, 
    tcp=0xffffffd00192cc88, ifqp=0xffffffc082ae0058)
    at /usr/rtm/symbsd/src/sys/netpfil/ipfilter/netinet/ip_state.c:3057
#5  0xffffffc0000b1f5e in ipf_state_check (fin=0x92, passp=0xffffffc082ae00cc)
    at /usr/rtm/symbsd/src/sys/netpfil/ipfilter/netinet/ip_state.c:3252
#6  0xffffffc00008988e in ipf_check (ctx=0xffffffc03decf000, 
    ip=<optimized out>, hlen=<optimized out>, ifp=<optimized out>, out=1, 
    mp=0xffffffc082ae0460)
    at /usr/rtm/symbsd/src/sys/netpfil/ipfilter/netinet/fil.c:2971
#7  0xffffffc000095d6e in ipf_check_wrapper6 (mp=0xffffffc082ae0460, ifp=0x3, 
    flags=<optimized out>, ruleset=<optimized out>, inp=<optimized out>)
    at /usr/rtm/symbsd/src/sys/netpfil/ipfilter/netinet/ip_fil_freebsd.c:137
#8  0xffffffc0005bdd9e in pfil_mbuf_common (pch=<optimized out>, 
    m=0xffffffc082ae0460, ifp=0xffffffd018c4d800, flags=131072, inp=0x0)
    at /usr/rtm/symbsd/src/sys/net/pfil.c:213
#9  pfil_mbuf_out (head=<optimized out>, m=0xffffffc082ae0460, 
    ifp=0xffffffd018c4d800, inp=0x0) at /usr/rtm/symbsd/src/sys/net/pfil.c:239
#10 0xffffffc00071e12c in ip6_output (m0=<optimized out>, opt=0x0, ro=0x0, 
    flags=<optimized out>, im6o=<optimized out>, ifpp=0xffffffc082ae0510, 
    inp=0x0) at /usr/rtm/symbsd/src/sys/netinet6/ip6_output.c:1027
#11 0xffffffc000703ea6 in icmp6_reflect (m=0xffffffd018b73500, 
    off=<optimized out>) at /usr/rtm/symbsd/src/sys/netinet6/icmp6.c:2171
#12 0xffffffc000703738 in icmp6_error (m=0xffffffd018b73500, 
    type=<optimized out>, code=0, param=0)
    at /usr/rtm/symbsd/src/sys/netinet6/icmp6.c:390
#13 0xffffffc000717790 in ip6_forward (m=0xffffffd018b73400, 
    srcrt=<optimized out>)
    at /usr/rtm/symbsd/src/sys/netinet6/ip6_forward.c:135
#14 0xffffffc000718e94 in ip6_input (m=0xffffffd018b73400)
    at /usr/rtm/symbsd/src/sys/netinet6/ip6_input.c:903

                        if (M_LEN(fin->fin_m) < fin->fin_plen) {

(gdb) print fin->fin_m
$14 = (mb_t *) 0x0
Comment 1 Mark Johnston freebsd_committer freebsd_triage 2025-07-25 16:32:50 UTC
Cy, do you have some time to take a look at this ipf bug?
Comment 2 Cy Schubert freebsd_committer freebsd_triage 2025-08-20 21:23:13 UTC
Confirmed on my sandbox machine. I have a good dump.
Comment 3 Cy Schubert freebsd_committer freebsd_triage 2025-08-20 21:32:39 UTC
It is not clear by looking at the upstream commit that added the block of code why nullifying the mbuf address as performed. It's part of a jumbo commit. Certainly no clue there.
Comment 4 commit-hook freebsd_committer freebsd_triage 2026-05-20 15:35:54 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=68ed81631afa20c07883f7f60343f6da8397ee41

commit 68ed81631afa20c07883f7f60343f6da8397ee41
Author:     Teddy Engel <engel.teddy@gmail.com>
AuthorDate: 2026-05-19 21:36:15 +0000
Commit:     Cy Schubert <cy@FreeBSD.org>
CommitDate: 2026-05-20 15:33:43 +0000

    ipfilter: Add NULL check for fin_dp in ICMP packet handlers

    Add NULL checks for fin->fin_dp in ipf_pr_icmp6() and ipf_pr_icmp()
    before dereferencing. When processing packets with IPv6 extension
    headers, ipf_pr_pullup() can succeed but fin->fin_dp may still be NULL
    due to extension header processing leaving insufficient data for the
    protocol header.

    PR:             288333
    MFC after:      1 week
    Pull Request:   https://github.com/freebsd/freebsd-src/pull/2214
    Signed-off-by:  Teddy Engel <engel.teddy@gmail.com>

 sys/netpfil/ipfilter/netinet/fil.c | 4 ++++
 1 file changed, 4 insertions(+)
Comment 5 commit-hook freebsd_committer freebsd_triage 2026-05-20 15:35:55 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=c028080749c09e68c555155df0e9f681ba63c6ae

commit c028080749c09e68c555155df0e9f681ba63c6ae
Author:     Teddy Engel <engel.teddy@gmail.com>
AuthorDate: 2026-05-19 21:36:33 +0000
Commit:     Cy Schubert <cy@FreeBSD.org>
CommitDate: 2026-05-20 15:33:43 +0000

    ipfilter: Fix NULL dereferences in ipf_checkicmp6matchingstate()

    Add NULL checks for ic6 (the ICMPv6 header pointer from fin->fin_dp)
    and oic (the inner ICMPv6 header from ofin.fin_dp after ipf_makefrip).
    These pointers can be NULL when processing malformed ICMPv6 error
    packets with extension headers.

    Also fix the length validation: the original check (fin->fin_plen <
    sizeof(ip6_t)) could never trigger because an earlier check already
    ensures fin->fin_plen >= ICMP6ERR_MINPKTLEN (48). Replace with a proper
    check that fin->fin_dlen contains at least ICMPERR_ICMPHLEN +
    sizeof(ip6_t) bytes to ensure sufficient data exists for both the
    ICMPv6 error header and the embedded IPv6 header.

    PR:             288333
    MFC after:      1 week
    Pull Request:   https://github.com/freebsd/freebsd-src/pull/2214
    Signed-off-by:  Teddy Engel <engel.teddy@gmail.com>

 sys/netpfil/ipfilter/netinet/ip_state.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)
Comment 6 commit-hook freebsd_committer freebsd_triage 2026-05-20 15:35:56 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=cdc40489a7a617b742e295cf9005b3569b45e823

commit cdc40489a7a617b742e295cf9005b3569b45e823
Author:     Teddy Engel <engel.teddy@gmail.com>
AuthorDate: 2026-05-19 21:36:23 +0000
Commit:     Cy Schubert <cy@FreeBSD.org>
CommitDate: 2026-05-20 15:33:43 +0000

    ipfilter: Add NULL check for fin_m in ipf_pr_icmp6()

    Add NULL check for fin->fin_m before calling M_LEN() in the ICMPv6
    error handling code path. When ipf_checkicmp6matchingstate() calls
    ipf_makefrip() with a synthesized fr_info_t that has fin_m set to
    NULL, the subsequent call to ipf_pr_ipv6hdr() can reach ipf_pr_icmp6()
    which would crash when trying to access the mbuf via M_LEN().

    PR:             288333
    MFC after:      1 week
    Pull Request:   https://github.com/freebsd/freebsd-src/pull/2214
    Signed-off-by:  Teddy Engel <engel.teddy@gmail.com>

 sys/netpfil/ipfilter/netinet/fil.c | 3 +++
 1 file changed, 3 insertions(+)
Comment 7 commit-hook freebsd_committer freebsd_triage 2026-05-27 13:42:02 UTC
A commit in branch stable/15 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=2349f5a91587eaf00c02ae49b0471cf191bec73a

commit 2349f5a91587eaf00c02ae49b0471cf191bec73a
Author:     Teddy Engel <engel.teddy@gmail.com>
AuthorDate: 2026-05-19 21:36:33 +0000
Commit:     Cy Schubert <cy@FreeBSD.org>
CommitDate: 2026-05-27 13:41:26 +0000

    ipfilter: Fix NULL dereferences in ipf_checkicmp6matchingstate()

    Add NULL checks for ic6 (the ICMPv6 header pointer from fin->fin_dp)
    and oic (the inner ICMPv6 header from ofin.fin_dp after ipf_makefrip).
    These pointers can be NULL when processing malformed ICMPv6 error
    packets with extension headers.

    Also fix the length validation: the original check (fin->fin_plen <
    sizeof(ip6_t)) could never trigger because an earlier check already
    ensures fin->fin_plen >= ICMP6ERR_MINPKTLEN (48). Replace with a proper
    check that fin->fin_dlen contains at least ICMPERR_ICMPHLEN +
    sizeof(ip6_t) bytes to ensure sufficient data exists for both the
    ICMPv6 error header and the embedded IPv6 header.

    PR:             288333
    MFC after:      1 week
    Pull Request:   https://github.com/freebsd/freebsd-src/pull/2214
    Signed-off-by:  Teddy Engel <engel.teddy@gmail.com>

    (cherry picked from commit c028080749c09e68c555155df0e9f681ba63c6ae)

 sys/netpfil/ipfilter/netinet/ip_state.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)
Comment 8 commit-hook freebsd_committer freebsd_triage 2026-05-27 13:42:03 UTC
A commit in branch stable/15 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=fefcd4994069de7fe0d4f94f2f9f9d3e5ae47ba2

commit fefcd4994069de7fe0d4f94f2f9f9d3e5ae47ba2
Author:     Teddy Engel <engel.teddy@gmail.com>
AuthorDate: 2026-05-19 21:36:23 +0000
Commit:     Cy Schubert <cy@FreeBSD.org>
CommitDate: 2026-05-27 13:41:25 +0000

    ipfilter: Add NULL check for fin_m in ipf_pr_icmp6()

    Add NULL check for fin->fin_m before calling M_LEN() in the ICMPv6
    error handling code path. When ipf_checkicmp6matchingstate() calls
    ipf_makefrip() with a synthesized fr_info_t that has fin_m set to
    NULL, the subsequent call to ipf_pr_ipv6hdr() can reach ipf_pr_icmp6()
    which would crash when trying to access the mbuf via M_LEN().

    PR:             288333
    MFC after:      1 week
    Pull Request:   https://github.com/freebsd/freebsd-src/pull/2214
    Signed-off-by:  Teddy Engel <engel.teddy@gmail.com>

    (cherry picked from commit cdc40489a7a617b742e295cf9005b3569b45e823)

 sys/netpfil/ipfilter/netinet/fil.c | 3 +++
 1 file changed, 3 insertions(+)
Comment 9 commit-hook freebsd_committer freebsd_triage 2026-05-27 13:42:04 UTC
A commit in branch stable/15 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=b2076f39a11723506f202f980fda281af3448bf4

commit b2076f39a11723506f202f980fda281af3448bf4
Author:     Teddy Engel <engel.teddy@gmail.com>
AuthorDate: 2026-05-19 21:36:15 +0000
Commit:     Cy Schubert <cy@FreeBSD.org>
CommitDate: 2026-05-27 13:41:25 +0000

    ipfilter: Add NULL check for fin_dp in ICMP packet handlers

    Add NULL checks for fin->fin_dp in ipf_pr_icmp6() and ipf_pr_icmp()
    before dereferencing. When processing packets with IPv6 extension
    headers, ipf_pr_pullup() can succeed but fin->fin_dp may still be NULL
    due to extension header processing leaving insufficient data for the
    protocol header.

    PR:             288333
    Pull Request:   https://github.com/freebsd/freebsd-src/pull/2214
    Signed-off-by:  Teddy Engel <engel.teddy@gmail.com>

    (cherry picked from commit 68ed81631afa20c07883f7f60343f6da8397ee41)

 sys/netpfil/ipfilter/netinet/fil.c | 4 ++++
 1 file changed, 4 insertions(+)
Comment 10 commit-hook freebsd_committer freebsd_triage 2026-05-27 13:43:06 UTC
A commit in branch stable/14 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=36ab14d7952997b102ca07ae2b61590b52144078

commit 36ab14d7952997b102ca07ae2b61590b52144078
Author:     Teddy Engel <engel.teddy@gmail.com>
AuthorDate: 2026-05-19 21:36:33 +0000
Commit:     Cy Schubert <cy@FreeBSD.org>
CommitDate: 2026-05-27 13:42:02 +0000

    ipfilter: Fix NULL dereferences in ipf_checkicmp6matchingstate()

    Add NULL checks for ic6 (the ICMPv6 header pointer from fin->fin_dp)
    and oic (the inner ICMPv6 header from ofin.fin_dp after ipf_makefrip).
    These pointers can be NULL when processing malformed ICMPv6 error
    packets with extension headers.

    Also fix the length validation: the original check (fin->fin_plen <
    sizeof(ip6_t)) could never trigger because an earlier check already
    ensures fin->fin_plen >= ICMP6ERR_MINPKTLEN (48). Replace with a proper
    check that fin->fin_dlen contains at least ICMPERR_ICMPHLEN +
    sizeof(ip6_t) bytes to ensure sufficient data exists for both the
    ICMPv6 error header and the embedded IPv6 header.

    PR:             288333
    Pull Request:   https://github.com/freebsd/freebsd-src/pull/2214
    Signed-off-by:  Teddy Engel <engel.teddy@gmail.com>

    (cherry picked from commit c028080749c09e68c555155df0e9f681ba63c6ae)

 sys/netpfil/ipfilter/netinet/ip_state.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)
Comment 11 commit-hook freebsd_committer freebsd_triage 2026-05-27 13:43:07 UTC
A commit in branch stable/14 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=fb0729bdc278d5402fc935998b5051d471c5380b

commit fb0729bdc278d5402fc935998b5051d471c5380b
Author:     Teddy Engel <engel.teddy@gmail.com>
AuthorDate: 2026-05-19 21:36:15 +0000
Commit:     Cy Schubert <cy@FreeBSD.org>
CommitDate: 2026-05-27 13:42:01 +0000

    ipfilter: Add NULL check for fin_dp in ICMP packet handlers

    Add NULL checks for fin->fin_dp in ipf_pr_icmp6() and ipf_pr_icmp()
    before dereferencing. When processing packets with IPv6 extension
    headers, ipf_pr_pullup() can succeed but fin->fin_dp may still be NULL
    due to extension header processing leaving insufficient data for the
    protocol header.

    PR:             288333
    Pull Request:   https://github.com/freebsd/freebsd-src/pull/2214
    Signed-off-by:  Teddy Engel <engel.teddy@gmail.com>

    (cherry picked from commit 68ed81631afa20c07883f7f60343f6da8397ee41)

 sys/netpfil/ipfilter/netinet/fil.c | 4 ++++
 1 file changed, 4 insertions(+)
Comment 12 commit-hook freebsd_committer freebsd_triage 2026-05-27 13:43:09 UTC
A commit in branch stable/14 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=a98f5fa642171f1ec89a4b13b23e0fc1c180b663

commit a98f5fa642171f1ec89a4b13b23e0fc1c180b663
Author:     Teddy Engel <engel.teddy@gmail.com>
AuthorDate: 2026-05-19 21:36:23 +0000
Commit:     Cy Schubert <cy@FreeBSD.org>
CommitDate: 2026-05-27 13:42:01 +0000

    ipfilter: Add NULL check for fin_m in ipf_pr_icmp6()

    Add NULL check for fin->fin_m before calling M_LEN() in the ICMPv6
    error handling code path. When ipf_checkicmp6matchingstate() calls
    ipf_makefrip() with a synthesized fr_info_t that has fin_m set to
    NULL, the subsequent call to ipf_pr_ipv6hdr() can reach ipf_pr_icmp6()
    which would crash when trying to access the mbuf via M_LEN().

    PR:             288333
    Pull Request:   https://github.com/freebsd/freebsd-src/pull/2214
    Signed-off-by:  Teddy Engel <engel.teddy@gmail.com>

    (cherry picked from commit cdc40489a7a617b742e295cf9005b3569b45e823)

 sys/netpfil/ipfilter/netinet/fil.c | 3 +++
 1 file changed, 3 insertions(+)