Linux Netfilter development
 help / color / mirror / Atom feed
* [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