* [PATCH net v2] netfilter: nf_conntrack_sip: fix OOB read in sip_skip_whitespace()
@ 2026-08-14 19:25 Joas Antonio dos Santos
2026-08-17 23:36 ` Pablo Neira Ayuso
0 siblings, 1 reply; 2+ messages in thread
From: Joas Antonio dos Santos @ 2026-08-14 19:25 UTC (permalink / raw)
To: pablo, fw; +Cc: netfilter-devel, coreteam
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 <joasantonio108@gmail.com>
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH net v2] netfilter: nf_conntrack_sip: fix OOB read in sip_skip_whitespace()
2026-08-14 19:25 [PATCH net v2] netfilter: nf_conntrack_sip: fix OOB read in sip_skip_whitespace() Joas Antonio dos Santos
@ 2026-08-17 23:36 ` Pablo Neira Ayuso
0 siblings, 0 replies; 2+ messages in thread
From: Pablo Neira Ayuso @ 2026-08-17 23:36 UTC (permalink / raw)
To: Joas Antonio dos Santos; +Cc: fw, netfilter-devel, coreteam
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 <joasantonio108@gmail.com>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-08-17 23:36 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-14 19:25 [PATCH net v2] netfilter: nf_conntrack_sip: fix OOB read in sip_skip_whitespace() Joas Antonio dos Santos
2026-08-17 23:36 ` Pablo Neira Ayuso
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox