All of lore.kernel.org
 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

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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.