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]>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.