Re: [PATCH net v2] netfilter: nf_conntrack_sip: fix OOB read in sip_skip_whitespace()
Pablo Neira Ayuso <[email protected]>
| Newsgroups | gmane.comp.security.firewalls.netfilter.devel |
|---|---|
| Message-ID | <aoOa8LIQN7uCFL2B@chamomile> |
Hi,
Could you please repost with a shorter commit description?
Thanks.
On Fri, Aug 14, 2026 at 12:25:32PM -0700, Joas Antonio dos Santos wrote:
> Resending with the correct tree tag in the subject (this fixes a bug, so it targets net, not net-next) -- apologies for the noise on the first message.
>
> Summary: sip_skip_whitespace() in net/netfilter/nf_conntrack_sip.c has two
> distinct "ran out of data" outcomes but only signals one of them to its
> caller. When every remaining byte up to the buffer limit is a space or
> tab, its own for-loop condition ends the loop and it returns dptr ==
> limit, instead of the NULL that its sibling sip_follow_continuation()
> returns on its own "no more data" path. ct_sip_get_header() only checks
> for the NULL case; the next statement dereferences *dptr before checking
> it against limit, causing a 1-byte out-of-bounds read.
>
> Affected version: Linux mainline, commit 3d6d817622b0a9721e3cc404df
> 3469171582be13 (2026-08-12), current as of this report. Verified
> identical (empty diff) against origin/master of torvalds/linux on
> 2026-08-13; no prior fix exists upstream.
>
> File / function: net/netfilter/nf_conntrack_sip.c, sip_skip_whitespace()
> (around line 422), consumed by ct_sip_get_header() (around line 493).
>
> Impact: out-of-bounds read of 1 byte past the end of the SIP payload
> buffer. Confirmed by AddressSanitizer against a userspace harness
> linking the unmodified, extracted parsing functions from this file.
> In the real call path (sip_help_tcp()), dptr/datalen come directly
> from the linearized skb with no slack: dptr = skb->data + dataoff,
> datalen = skb->len - dataoff, so limit is exactly skb->data + skb->len,
> the true end of packet data -- unlike some other conntrack helpers
> that operate on a slightly larger scratch buffer, there is no spare
> byte here. Impact is a potential crash (denial of service) if the
> byte read falls on an unmapped page.
>
> Conditions: requires the nf_conntrack_sip helper to be attached to a
> TCP/UDP flow on the SIP port (5060 by default, or any port configured
> via the "ports" module parameter) -- a commonly enabled helper on
> routers/firewalls doing VoIP/SIP NAT. No authentication or established
> call state is required; ct_sip_get_header() is the primary header
> lookup used throughout SIP request/response and SDP processing, called
> for every recognized header (Via, From, To, Contact, CSeq, Expires,
> Content-Length, Call-Id).
>
> Reproducer: a 7-byte input triggers the bug directly against
> ct_sip_get_header(): a recognized header name followed only by
> spaces/tabs running exactly to the end of the payload, no colon
> present. Tested against the exact kernel source (functions extracted
> verbatim, unmodified, from net/netfilter/nf_conntrack_sip.c) compiled
> into a small userspace harness, reusing the kernel's own
> in4_pton()/in6_pton()/hex_to_bin() (net/core/utils.c, lib/hexdump.c)
> rather than reimplementing them. AddressSanitizer reports:
>
> heap-buffer-overflow, READ of size 1
> #0 ct_sip_get_header nf_conntrack_sip.c:493
>
> Also reproduced organically via libFuzzer+ASAN against the same
> extracted functions (ct_sip_parse_request, ct_sip_get_header,
> ct_sip_parse_header_uri, ct_sip_parse_param, ct_sip_parse_address_param,
> ct_sip_parse_numerical_param, ct_sip_get_sdp_header,
> ct_sip_parse_sdp_addr), from a well-formed INVITE seed within ~2200
> executions.
>
> The reproducer bytes are not attached to this message; available on
> request.
>
> Details: sip_skip_whitespace() is:
>
> static const char *sip_skip_whitespace(const char *dptr, const char *limit)
> {
> for (; dptr < limit; dptr++) {
> if (*dptr == ' ' || *dptr == '\t')
> continue;
> if (*dptr != '\r' && *dptr != '\n')
> break;
> dptr = sip_follow_continuation(dptr, limit);
> break;
> }
> return dptr;
> }
>
> If every byte from dptr to limit is a space/tab, the for loop exits via
> its own condition with dptr == limit, which is returned as-is -- a
> different "no more data" signal than the NULL returned by
> sip_follow_continuation() on its own exhausted-input path. The caller,
> ct_sip_get_header(), only guards the NULL case:
>
> dptr = sip_skip_whitespace(dptr, limit);
> if (dptr == NULL)
> break;
> if (*dptr != ':' || ++dptr >= limit) /* OOB read when dptr == limit */
> break;
>
> Before reporting, I checked every netfilter-devel thread touching
> nf_conntrack_sip.c from 2026-03 through 2026-08 (~15 threads, via
> marc.info since patchwork.ozlabs.org sits behind an anti-bot
> challenge), plus targeted searches for ct_sip_get_header and
> sip_skip_whitespace. None address this issue. Summary of what does
> exist, all distinct root causes, all already merged or in unrelated
> threads: port parsing after the address in sip_parse_port (merged,
> present in current mainline); a datalen integer-wraparound in
> sip_help_tcp()'s NAT rewrite-delta computation (KASAN symptom lands in
> ct_sip_get_header, but the root cause is in the caller); a NULL
> skb_dst() crash in set_expected_rtp_rtcp() under tc-ingress/openvswitch
> (two patches, unrelated SDP external-media code path); a memory-leak
> fix around nf_ct_expect_alloc() ordering in process_register_request();
> a cosmetic u_int*_t -> standard-types rename. None of these touch
> sip_skip_whitespace() or the dptr check that follows it in
> ct_sip_get_header().
>
> Proposed fix (tested: reproducer no longer crashes under ASAN;
> 500000-iteration fuzz regression run clean, no behavior change on the
> existing corpus): make sip_skip_whitespace() report both "no more data"
> outcomes the same way, matching the convention its own
> sip_follow_continuation() already uses and that both existing call
> sites already check for:
>
> --- a/net/netfilter/nf_conntrack_sip.c
> +++ b/net/netfilter/nf_conntrack_sip.c
> @@ -429,7 +429,7 @@ static const char *sip_skip_whitespace(const char *dptr, const char *limit)
> dptr = sip_follow_continuation(dptr, limit);
> break;
> }
> - return dptr;
> + return dptr < limit ? dptr : NULL;
> }
>
> Fixes: ea45f12a2766d ("[NETFILTER]: nf_conntrack_sip: parse SIP headers properly")
> Signed-off-by: Joas Antonio dos Santos <[email protected]>