{"id":2226531,"url":"http://patchwork.ozlabs.org/api/patches/2226531/?format=json","web_url":"http://patchwork.ozlabs.org/project/netfilter-devel/patch/20260422144057.14763-1-fw@strlen.de/","project":{"id":26,"url":"http://patchwork.ozlabs.org/api/projects/26/?format=json","name":"Netfilter Development","link_name":"netfilter-devel","list_id":"netfilter-devel.vger.kernel.org","list_email":"netfilter-devel@vger.kernel.org","web_url":null,"scm_url":null,"webscm_url":null,"list_archive_url":"","list_archive_url_format":"","commit_url_format":""},"msgid":"<20260422144057.14763-1-fw@strlen.de>","list_archive_url":null,"date":"2026-04-22T14:40:51","name":"[v2,nf] netfilter: nf_conntrack_sip: don't use simple_strtoul","commit_ref":null,"pull_url":null,"state":"changes-requested","archived":true,"hash":"a27009d0e4f77b18c2ef0e56d41b79b007880e18","submitter":{"id":1025,"url":"http://patchwork.ozlabs.org/api/people/1025/?format=json","name":"Florian Westphal","email":"fw@strlen.de"},"delegate":null,"mbox":"http://patchwork.ozlabs.org/project/netfilter-devel/patch/20260422144057.14763-1-fw@strlen.de/mbox/","series":[{"id":501026,"url":"http://patchwork.ozlabs.org/api/series/501026/?format=json","web_url":"http://patchwork.ozlabs.org/project/netfilter-devel/list/?series=501026","date":"2026-04-22T14:40:51","name":"[v2,nf] netfilter: nf_conntrack_sip: don't use simple_strtoul","version":2,"mbox":"http://patchwork.ozlabs.org/series/501026/mbox/"}],"comments":"http://patchwork.ozlabs.org/api/patches/2226531/comments/","check":"pending","checks":"http://patchwork.ozlabs.org/api/patches/2226531/checks/","tags":{},"related":[],"headers":{"Return-Path":"\n <netfilter-devel+bounces-12137-incoming=patchwork.ozlabs.org@vger.kernel.org>","X-Original-To":["incoming@patchwork.ozlabs.org","netfilter-devel@vger.kernel.org"],"Delivered-To":"patchwork-incoming@legolas.ozlabs.org","Authentication-Results":["legolas.ozlabs.org;\n spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org\n (client-ip=2600:3c0a:e001:db::12fc:5321; helo=sea.lore.kernel.org;\n envelope-from=netfilter-devel+bounces-12137-incoming=patchwork.ozlabs.org@vger.kernel.org;\n receiver=patchwork.ozlabs.org)","smtp.subspace.kernel.org;\n arc=none smtp.client-ip=91.216.245.30","smtp.subspace.kernel.org;\n dmarc=none (p=none dis=none) header.from=strlen.de","smtp.subspace.kernel.org;\n spf=pass smtp.mailfrom=Chamillionaire.breakpoint.cc"],"Received":["from sea.lore.kernel.org (sea.lore.kernel.org\n [IPv6:2600:3c0a:e001:db::12fc:5321])\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)\n\t key-exchange x25519)\n\t(No client certificate requested)\n\tby legolas.ozlabs.org (Postfix) with ESMTPS id 4g129T27v6z1y2d\n\tfor <incoming@patchwork.ozlabs.org>; Thu, 23 Apr 2026 00:46:37 +1000 (AEST)","from smtp.subspace.kernel.org (conduit.subspace.kernel.org\n [100.90.174.1])\n\tby sea.lore.kernel.org (Postfix) with ESMTP id 823C930648B2\n\tfor <incoming@patchwork.ozlabs.org>; Wed, 22 Apr 2026 14:41:08 +0000 (UTC)","from localhost.localdomain (localhost.localdomain [127.0.0.1])\n\tby smtp.subspace.kernel.org (Postfix) with ESMTP id C395A3ED5D4;\n\tWed, 22 Apr 2026 14:41:07 +0000 (UTC)","from Chamillionaire.breakpoint.cc (Chamillionaire.breakpoint.cc\n [91.216.245.30])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))\n\t(No client certificate requested)\n\tby smtp.subspace.kernel.org (Postfix) with ESMTPS id 85A893ED5CF\n\tfor <netfilter-devel@vger.kernel.org>; Wed, 22 Apr 2026 14:41:05 +0000 (UTC)","by Chamillionaire.breakpoint.cc (Postfix, from userid 1003)\n\tid E516660890; Wed, 22 Apr 2026 16:41:02 +0200 (CEST)"],"ARC-Seal":"i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116;\n\tt=1776868867; cv=none;\n b=S80Z3ZmpGjw7G7tw9Wso6oLScGA51wWad1M7qSLD35CytR1knwC7HIYDklovR3PBiweuGOWfOam8ZawJCFFMBsJNB6b86rt0QRz03/y0OYWJFdAp3YPBVX/FuDtJYKdTVTMsm0M0T+CwdqZjNv4kJXJI09ethMRQ//rpZIq+t/0=","ARC-Message-Signature":"i=1; a=rsa-sha256; d=subspace.kernel.org;\n\ts=arc-20240116; t=1776868867; c=relaxed/simple;\n\tbh=mwDxI2VhfDkjQMkhjBgiNtDNs+Gs1vHB/dr7FA0ev00=;\n\th=From:To:Subject:Date:Message-ID:MIME-Version:Content-Type;\n b=Nbh9/X+p34ttO4tYSuM0lQH4c6stPmCLELLButqXPc48DjqlyS5FxdTgCyEcrIi+QdSHt0FaG5aFKXF17itRCJDYMVbiZuVnEHcPNIM8czwdrBRnEIl4Iyb7QjuFUXxZdrxxT4xhvpDWGsJOnlfqThMWvtkHhHmB6MuGSUqA1wI=","ARC-Authentication-Results":"i=1; smtp.subspace.kernel.org;\n dmarc=none (p=none dis=none) header.from=strlen.de;\n spf=pass smtp.mailfrom=Chamillionaire.breakpoint.cc;\n arc=none smtp.client-ip=91.216.245.30","From":"Florian Westphal <fw@strlen.de>","To":"<netfilter-devel@vger.kernel.org>","Subject":"[PATCH v2 nf] netfilter: nf_conntrack_sip: don't use simple_strtoul","Date":"Wed, 22 Apr 2026 16:40:51 +0200","Message-ID":"<20260422144057.14763-1-fw@strlen.de>","X-Mailer":"git-send-email 2.53.0","Precedence":"bulk","X-Mailing-List":"netfilter-devel@vger.kernel.org","List-Id":"<netfilter-devel.vger.kernel.org>","List-Subscribe":"<mailto:netfilter-devel+subscribe@vger.kernel.org>","List-Unsubscribe":"<mailto:netfilter-devel+unsubscribe@vger.kernel.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=UTF-8","Content-Transfer-Encoding":"8bit"},"content":"Replace unsafe port parsing in epaddr_len(), ct_sip_parse_header_uri(),\nand ct_sip_parse_request() with a new sip_parse_port() helper that\nvalidates each digit against the buffer limit, eliminating the use of\nsimple_strtoul() which assumes NUL-terminated strings.\n\nThe previous code dereferenced pointers without bounds checks after\nsip_parse_addr() and relied on simple_strtoul() on non-NUL-terminated\nskb data. A port that reaches the buffer limit without a trailing\ncharacter is also rejected as malformed.\n\nAlso get rid of all simple_strtoul() usage in conntrack, prefer a\nstricter version instead.  There are intentional changes:\n\n- Bail out if number is > UINT_MAX and indicate a failure, same for\n  too long sequences.\n  While we do accept 05535 as port 5535, we will not accept e.g.\n  'sip:10.0.0.1:005060'.  While its syntactically valid under RFC 3261,\n  we should restrict this to not waste cycles when presented with\n  malformed packets with 64k '0' characters.\n\n- Force base 10 in ct_sip_parse_numerical_param(). This is used to fetch\n  'expire=' and 'rports='; both are expected to use base-10.\n\n- In nf_nat_sip.c, only accept the parsed value if its within the 1k-64k\n  range.\n\n- epaddr_len now returns 0 if the port is invalid, as it already does\n  for invalid ip addresses.  This is intentional. nf_conntrack_sip\n  performs lots of guesswork to find the right parts of the message\n  to parse.  Being stricter could break existing setups.\n  Connection tracking helpers are designed to allow traffic to\n  pass, not to block it.\n\nBased on an earlier patch from Jenny Guanni Qu <qguanni@gmail.com>.\n\nFixes: 05e3ced297fe (\"[NETFILTER]: nf_conntrack_sip: introduce SIP-URI parsing helper\")\nReported-by: Klaudia Kloc <klaudia@vidocsecurity.com>\nReported-by: Dawid Moczadło <dawid@vidocsecurity.com>\nReported-by: Jenny Guanni Qu <qguanni@gmail.com>.\nSigned-off-by: Florian Westphal <fw@strlen.de>\n---\n v2: supersedes all inflight sip patches into one single changeset.\n\n net/netfilter/nf_conntrack_sip.c | 149 ++++++++++++++++++++++++-------\n net/netfilter/nf_nat_sip.c       |   1 +\n 2 files changed, 117 insertions(+), 33 deletions(-)","diff":"diff --git a/net/netfilter/nf_conntrack_sip.c b/net/netfilter/nf_conntrack_sip.c\nindex 182cfb119448..aa5da83c74f0 100644\n--- a/net/netfilter/nf_conntrack_sip.c\n+++ b/net/netfilter/nf_conntrack_sip.c\n@@ -181,6 +181,57 @@ static int sip_parse_addr(const struct nf_conn *ct, const char *cp,\n \treturn 1;\n }\n \n+/* Parse optional port number after IP address.\n+ * Returns false on malformed input, true otherwise.\n+ * If port is non-NULL, stores parsed port in network byte order.\n+ * If no port is present, sets *port to default SIP port.\n+ */\n+static bool sip_parse_port(const char *dptr, const char **endp,\n+\t\t\t   const char *limit, __be16 *port)\n+{\n+\tunsigned int p = 0;\n+\tint len = 0;\n+\n+\tif (dptr >= limit)\n+\t\treturn false;\n+\n+\tif (*dptr != ':') {\n+\t\tif (port)\n+\t\t\t*port = htons(SIP_PORT);\n+\t\tif (endp)\n+\t\t\t*endp = dptr;\n+\t\treturn true;\n+\t}\n+\n+\tdptr++; /* skip ':' */\n+\n+\twhile (dptr < limit && isdigit(*dptr)) {\n+\t\tp = p * 10 + (*dptr - '0');\n+\t\tdptr++;\n+\t\tlen++;\n+\t\tif (len > 5) /* max \"65535\" */\n+\t\t\treturn false;\n+\t}\n+\n+\tif (len == 0)\n+\t\treturn false;\n+\n+\t/* reached limit while parsing port */\n+\tif (dptr >= limit)\n+\t\treturn false;\n+\n+\tif (p < 1024 || p > 65535)\n+\t\treturn false;\n+\n+\tif (port)\n+\t\t*port = htons(p);\n+\n+\tif (endp)\n+\t\t*endp = dptr;\n+\n+\treturn true;\n+}\n+\n /* skip ip address. returns its length. */\n static int epaddr_len(const struct nf_conn *ct, const char *dptr,\n \t\t      const char *limit, int *shift)\n@@ -193,11 +244,8 @@ static int epaddr_len(const struct nf_conn *ct, const char *dptr,\n \t\treturn 0;\n \t}\n \n-\t/* Port number */\n-\tif (*dptr == ':') {\n-\t\tdptr++;\n-\t\tdptr += digits_len(ct, dptr, limit, shift);\n-\t}\n+\tif (!sip_parse_port(dptr, &dptr, limit, NULL))\n+\t\treturn 0;\n \treturn dptr - aux;\n }\n \n@@ -228,6 +276,51 @@ static int skp_epaddr_len(const struct nf_conn *ct, const char *dptr,\n \treturn epaddr_len(ct, dptr, limit, shift);\n }\n \n+/* simple_strtoul stops after first non-number character.\n+ * But as we're not dealing with c-strings, we can't rely on\n+ * hitting \\r,\\n,\\0 etc. before moving past end of buffer.\n+ *\n+ * This is a variant of simple_strtoul, but doesn't require\n+ * a c-string.\n+ *\n+ * If value exceeds UINT_MAX, 0 is returned.\n+ */\n+static unsigned int sip_strtouint(const char *cp, unsigned int len, char **endp)\n+{\n+\tconst unsigned int max = sizeof(\"4294967295\");\n+\tunsigned int olen = len;\n+\tconst char *s = cp;\n+\tu64 result = 0;\n+\n+\tif (len > max)\n+\t\tlen = max;\n+\n+\twhile (olen > 0 && isdigit(*s)) {\n+\t\tunsigned int value;\n+\n+\t\tif (len == 0)\n+\t\t\tgoto err;\n+\n+\t\tvalue = *s - '0';\n+\t\tresult = result * 10 + value;\n+\n+\t\tif (result > UINT_MAX)\n+\t\t\tgoto err;\n+\t\ts++;\n+\t\tlen--;\n+\t\tolen--;\n+\t}\n+\n+\tif (endp)\n+\t\t*endp = (char *)s;\n+\n+\treturn result;\n+err:\n+\tif (endp)\n+\t\t*endp = (char *)cp;\n+\treturn 0;\n+}\n+\n /* Parse a SIP request line of the form:\n  *\n  * Request-Line = Method SP Request-URI SP SIP-Version CRLF\n@@ -241,7 +334,6 @@ int ct_sip_parse_request(const struct nf_conn *ct,\n {\n \tconst char *start = dptr, *limit = dptr + datalen, *end;\n \tunsigned int mlen;\n-\tunsigned int p;\n \tint shift = 0;\n \n \t/* Skip method and following whitespace */\n@@ -267,14 +359,8 @@ int ct_sip_parse_request(const struct nf_conn *ct,\n \n \tif (!sip_parse_addr(ct, dptr, &end, addr, limit, true))\n \t\treturn -1;\n-\tif (end < limit && *end == ':') {\n-\t\tend++;\n-\t\tp = simple_strtoul(end, (char **)&end, 10);\n-\t\tif (p < 1024 || p > 65535)\n-\t\t\treturn -1;\n-\t\t*port = htons(p);\n-\t} else\n-\t\t*port = htons(SIP_PORT);\n+\tif (!sip_parse_port(end, &end, limit, port))\n+\t\treturn -1;\n \n \tif (end == dptr)\n \t\treturn 0;\n@@ -509,7 +595,6 @@ int ct_sip_parse_header_uri(const struct nf_conn *ct, const char *dptr,\n \t\t\t    union nf_inet_addr *addr, __be16 *port)\n {\n \tconst char *c, *limit = dptr + datalen;\n-\tunsigned int p;\n \tint ret;\n \n \tret = ct_sip_walk_headers(ct, dptr, dataoff ? *dataoff : 0, datalen,\n@@ -520,14 +605,8 @@ int ct_sip_parse_header_uri(const struct nf_conn *ct, const char *dptr,\n \n \tif (!sip_parse_addr(ct, dptr + *matchoff, &c, addr, limit, true))\n \t\treturn -1;\n-\tif (*c == ':') {\n-\t\tc++;\n-\t\tp = simple_strtoul(c, (char **)&c, 10);\n-\t\tif (p < 1024 || p > 65535)\n-\t\t\treturn -1;\n-\t\t*port = htons(p);\n-\t} else\n-\t\t*port = htons(SIP_PORT);\n+\tif (!sip_parse_port(c, &c, limit, port))\n+\t\treturn -1;\n \n \tif (dataoff)\n \t\t*dataoff = c - dptr;\n@@ -609,7 +688,7 @@ int ct_sip_parse_numerical_param(const struct nf_conn *ct, const char *dptr,\n \t\treturn 0;\n \n \tstart += strlen(name);\n-\t*val = simple_strtoul(start, &end, 0);\n+\t*val = sip_strtouint(start, limit - start, (char **)&end);\n \tif (start == end)\n \t\treturn -1;\n \tif (matchoff && matchlen) {\n@@ -1064,6 +1143,8 @@ static int process_sdp(struct sk_buff *skb, unsigned int protoff,\n \n \tmediaoff = sdpoff;\n \tfor (i = 0; i < ARRAY_SIZE(sdp_media_types); ) {\n+\t\tchar *end;\n+\n \t\tif (ct_sip_get_sdp_header(ct, *dptr, mediaoff, *datalen,\n \t\t\t\t\t  SDP_HDR_MEDIA, SDP_HDR_UNSPEC,\n \t\t\t\t\t  &mediaoff, &medialen) <= 0)\n@@ -1079,8 +1160,8 @@ static int process_sdp(struct sk_buff *skb, unsigned int protoff,\n \t\tmediaoff += t->len;\n \t\tmedialen -= t->len;\n \n-\t\tport = simple_strtoul(*dptr + mediaoff, NULL, 10);\n-\t\tif (port == 0)\n+\t\tport = sip_strtouint(*dptr + mediaoff, *datalen - mediaoff, (char **)&end);\n+\t\tif (port == 0 || *dptr + mediaoff == end)\n \t\t\tcontinue;\n \t\tif (port < 1024 || port > 65535) {\n \t\t\tnf_ct_helper_log(skb, ct, \"wrong port %u\", port);\n@@ -1254,7 +1335,7 @@ static int process_register_request(struct sk_buff *skb, unsigned int protoff,\n \t */\n \tif (ct_sip_get_header(ct, *dptr, 0, *datalen, SIP_HDR_EXPIRES,\n \t\t\t      &matchoff, &matchlen) > 0)\n-\t\texpires = simple_strtoul(*dptr + matchoff, NULL, 10);\n+\t\texpires = sip_strtouint(*dptr + matchoff, *datalen - matchoff, NULL);\n \n \tret = ct_sip_parse_header_uri(ct, *dptr, NULL, *datalen,\n \t\t\t\t      SIP_HDR_CONTACT, NULL,\n@@ -1358,7 +1439,7 @@ static int process_register_response(struct sk_buff *skb, unsigned int protoff,\n \n \tif (ct_sip_get_header(ct, *dptr, 0, *datalen, SIP_HDR_EXPIRES,\n \t\t\t      &matchoff, &matchlen) > 0)\n-\t\texpires = simple_strtoul(*dptr + matchoff, NULL, 10);\n+\t\texpires = sip_strtouint(*dptr + matchoff, *datalen - matchoff, NULL);\n \n \twhile (1) {\n \t\tunsigned int c_expires = expires;\n@@ -1421,7 +1502,8 @@ static int process_sip_response(struct sk_buff *skb, unsigned int protoff,\n \n \tif (*datalen < strlen(\"SIP/2.0 200\"))\n \t\treturn NF_ACCEPT;\n-\tcode = simple_strtoul(*dptr + strlen(\"SIP/2.0 \"), NULL, 10);\n+\tcode = sip_strtouint(*dptr + strlen(\"SIP/2.0 \"),\n+\t\t\t     *datalen - strlen(\"SIP/2.0\"), NULL);\n \tif (!code) {\n \t\tnf_ct_helper_log(skb, ct, \"cannot get code\");\n \t\treturn NF_DROP;\n@@ -1432,7 +1514,7 @@ static int process_sip_response(struct sk_buff *skb, unsigned int protoff,\n \t\tnf_ct_helper_log(skb, ct, \"cannot parse cseq\");\n \t\treturn NF_DROP;\n \t}\n-\tcseq = simple_strtoul(*dptr + matchoff, NULL, 10);\n+\tcseq = sip_strtouint(*dptr + matchoff, *datalen - matchoff, NULL);\n \tif (!cseq && *(*dptr + matchoff) != '0') {\n \t\tnf_ct_helper_log(skb, ct, \"cannot get cseq\");\n \t\treturn NF_DROP;\n@@ -1482,6 +1564,7 @@ static int process_sip_request(struct sk_buff *skb, unsigned int protoff,\n \n \tfor (i = 0; i < ARRAY_SIZE(sip_handlers); i++) {\n \t\tconst struct sip_handler *handler;\n+\t\tchar *end;\n \n \t\thandler = &sip_handlers[i];\n \t\tif (handler->request == NULL)\n@@ -1498,8 +1581,8 @@ static int process_sip_request(struct sk_buff *skb, unsigned int protoff,\n \t\t\tnf_ct_helper_log(skb, ct, \"cannot parse cseq\");\n \t\t\treturn NF_DROP;\n \t\t}\n-\t\tcseq = simple_strtoul(*dptr + matchoff, NULL, 10);\n-\t\tif (!cseq && *(*dptr + matchoff) != '0') {\n+\t\tcseq = sip_strtouint(*dptr + matchoff, *datalen - matchoff, (char **)&end);\n+\t\tif (*dptr + matchoff == end) {\n \t\t\tnf_ct_helper_log(skb, ct, \"cannot get cseq\");\n \t\t\treturn NF_DROP;\n \t\t}\n@@ -1575,7 +1658,7 @@ static int sip_help_tcp(struct sk_buff *skb, unsigned int protoff,\n \t\t\t\t      &matchoff, &matchlen) <= 0)\n \t\t\tbreak;\n \n-\t\tclen = simple_strtoul(dptr + matchoff, (char **)&end, 10);\n+\t\tclen = sip_strtouint(dptr + matchoff, datalen - matchoff, (char **)&end);\n \t\tif (dptr + matchoff == end)\n \t\t\tbreak;\n \ndiff --git a/net/netfilter/nf_nat_sip.c b/net/netfilter/nf_nat_sip.c\nindex c845b6d1a2bd..9fbfc6bff0c2 100644\n--- a/net/netfilter/nf_nat_sip.c\n+++ b/net/netfilter/nf_nat_sip.c\n@@ -246,6 +246,7 @@ static unsigned int nf_nat_sip(struct sk_buff *skb, unsigned int protoff,\n \t\tif (ct_sip_parse_numerical_param(ct, *dptr, matchend, *datalen,\n \t\t\t\t\t\t \"rport=\", &poff, &plen,\n \t\t\t\t\t\t &n) > 0 &&\n+\t\t    n >= 1024 && n <= 65535 &&\n \t\t    htons(n) == ct->tuplehash[dir].tuple.dst.u.udp.port &&\n \t\t    htons(n) != ct->tuplehash[!dir].tuple.src.u.udp.port) {\n \t\t\t__be16 p = ct->tuplehash[!dir].tuple.src.u.udp.port;\n","prefixes":["v2","nf"]}