[PATCH] netfilter: conntrack_irc: fix port value truncation in parse_dcc()

[PATCH] netfilter: conntrack_irc: fix port value truncation in parse_dcc()

[Aamir Ahmed]

parse_dcc() uses simple_strtoul() to parse the port number from DCC
commands, storing the result directly in a u16.  simple_strtoul()
returns unsigned long, so values above 65535 are silently truncated.
For example, a DCC command with port "65537" is parsed as port 1,
bypassing the dcc_port == 0 rejection check and creating a conntrack
expectation for an unintended port.

The nf_conntrack_ftp helper had the same class of defect and was fixed
by using a wider accumulator with a range check (get_port checks
tmp_port > 65535).  The ipvs ftp helper was similarly fixed in commit
e625a9477d12 ("ipvs: fix integer overflow in ftp helper port/address
parsing").  Apply the same pattern here: parse into an unsigned long,
reject values above 65535, and only then assign to the u16.

Fixes: 869f37d8e48f ("[NETFILTER]: nf_conntrack/nf_nat: add IRC helper port")
Signed-off-by: Aamir Ahmed <elb12345@hotmail.co.uk>
---
 net/netfilter/nf_conntrack_irc.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/net/netfilter/nf_conntrack_irc.c b/net/netfilter/nf_conntrack_irc.c
index 92360963757a..8321f12dbf15 100644
--- a/net/netfilter/nf_conntrack_irc.c
+++ b/net/netfilter/nf_conntrack_irc.c
@@ -64,6 +64,7 @@ static int parse_dcc(char *data, const char *data_end, __be32 *ip,
 		     u_int16_t *port, char **ad_beg_p, char **ad_end_p)
 {
 	char *tmp;
+	unsigned long tmp_port;
 
 	/* at least 12: "AAAAAAAA P\1\n" */
 	while (*data++ != ' ')
@@ -88,7 +89,10 @@ static int parse_dcc(char *data, const char *data_end, __be32 *ip,
 		data++;
 	}
 
-	*port = simple_strtoul(data, &data, 10);
+	tmp_port = simple_strtoul(data, &data, 10);
+	if (tmp_port > 65535)
+		return -1;
+	*port = tmp_port;
 	*ad_end_p = data;
 
 	return 0;
-- 
2.43.0

Re: [PATCH] netfilter: conntrack_irc: fix port value truncation in parse_dcc()

[Florian Westphal]

Aamir Ahmed <elb12345@hotmail.co.uk> wrote:
> diff --git a/net/netfilter/nf_conntrack_irc.c b/net/netfilter/nf_conntrack_irc.c
> index 92360963757a..8321f12dbf15 100644
> --- a/net/netfilter/nf_conntrack_irc.c
> +++ b/net/netfilter/nf_conntrack_irc.c
> @@ -64,6 +64,7 @@ static int parse_dcc(char *data, const char *data_end, __be32 *ip,
>  		     u_int16_t *port, char **ad_beg_p, char **ad_end_p)
>  {
>  	char *tmp;
> +	unsigned long tmp_port;
>  
>  	/* at least 12: "AAAAAAAA P\1\n" */
>  	while (*data++ != ' ')
> @@ -88,7 +89,10 @@ static int parse_dcc(char *data, const char *data_end, __be32 *ip,
>  		data++;
>  	}
>  
> -	*port = simple_strtoul(data, &data, 10);
> +	tmp_port = simple_strtoul(data, &data, 10);
> +	if (tmp_port > 65535)
> +		return -1;

Your other patch also rejects port == 0.
Maybe also check and refuse for tmp_port < 1024 here.

This patch can be handled via nf-next, I think.

Re: [PATCH] netfilter: conntrack_irc: fix port value truncation in parse_dcc()

[Aamir Ahmed]

On Mon, Sep 07, 2026 at 10:32:17AM +0200, Florian Westphal wrote:
> Your other patch also rejects port == 0.
> Maybe also check and refuse for tmp_port < 1024 here.

Makes sense - the expectation is source-wildcarded and the DCC port is
the client's own listener, so a privileged port there is not something
the helper should open. v2 will reject tmp_port < 1024 || > 65535 and
drop the now-redundant dcc_port == 0 test in help() (or I can put the
floor there instead, so the forged-DCC warning still fires - your
preference). I will describe it as a behaviour change with your
Suggested-by rather than part of the truncation fix.

While respinning I noticed simple_strtoul() narrows to 32 bits before
the range check on 32-bit, so v2 will use simple_strtoull().

> This patch can be handled via nf-next, I think.

Will resend as [PATCH nf-next v2], without the stable Cc.

Kind Regards

Aamir A.

Re: [PATCH] netfilter: conntrack_irc: fix port value truncation in parse_dcc()

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider:

Critical: 0 · High: 1 · Medium: 0 · Low: 0

- [High] Incomplete fix: in parse_dcc()
  (net/netfilter/nf_conntrack_irc.c:92-95) the new bound check is…

--

netfilter: conntrack_irc: fix port value truncation in parse_dcc()

This adds a bound check to the DCC port parsing in parse_dcc() so that
values above 65535 are rejected instead of being silently truncated into
the u16 output.  The stated goal is to prevent a conntrack expectation
being created for a port that never appeared in the DCC command.

> diff --git a/net/netfilter/nf_conntrack_irc.c b/net/netfilter/nf_conntrack_irc.c
> index 92360963757a9..8321f12dbf155 100644
> --- a/net/netfilter/nf_conntrack_irc.c
> +++ b/net/netfilter/nf_conntrack_irc.c
> @@ -64,6 +64,7 @@ static int parse_dcc(char *data, const char *data_end, __be32 *ip,
>  		     u_int16_t *port, char **ad_beg_p, char **ad_end_p)
>  {
>  	char *tmp;
> +	unsigned long tmp_port;

[Severity: High]
Is unsigned long wide enough here on 32-bit builds?

> @@ -88,7 +89,10 @@ static int parse_dcc(char *data, const char *data_end, __be32 *ip,
>  		data++;
>  	}
>  
> -	*port = simple_strtoul(data, &data, 10);
> +	tmp_port = simple_strtoul(data, &data, 10);
> +	if (tmp_port > 65535)
> +		return -1;
> +	*port = tmp_port;

Does the truncation still happen before the new check runs on ILP32
(i386, ARM32, MIPS32), where unsigned long is 32 bits?

simple_strtoul() itself narrows:

lib/vsprintf.c:
unsigned long simple_strtoul(const char *cp, char **endp, unsigned int base)
{
	return simple_strtoull(cp, endp, base);
}

And _parse_integer_limit() in lib/kstrtox.c only saturates to ULLONG_MAX
when check_mul_overflow()/check_add_overflow() fire, so a value in the
2^32 range is returned exactly as a 64-bit result:

		if (unlikely(res & (~0ull << 60))) {
			if (check_mul_overflow(res, base, &res) ||
			    check_add_overflow(res, val, &res)) {
				res = ULLONG_MAX;
				overflow = KSTRTOX_OVERFLOW;

So for a DCC field of "4294967297", would the 64-bit 0x1_00000001 be
narrowed to 1 by the assignment into tmp_port, making tmp_port > 65535
false and leaving *port == 1?

If so, the path in help() would still be reached with a port that was
never in the command, since the only remaining guard is dcc_port == 0:

net/netfilter/nf_conntrack_irc.c:help() {
	...
			    dcc_port == 0) {
	...
			port = htons(dcc_port);
			nf_ct_expect_init(exp, NF_CT_EXPECT_CLASS_DEFAULT,
					  tuple->src.l3num,
					  NULL, &tuple->dst.u3,
					  IPPROTO_TCP, NULL, &port);
	...
}

The input here is remote IRC payload copied into irc_buffer by help(),
and parse_dcc() only requires a '\n' inside the buffer, so a payload of
the form "PRIVMSG x :\1DCC SEND t <ip> 4294967297\1\n" reaches this
parse.

The commit message says this follows nf_conntrack_ftp:

  "The nf_conntrack_ftp helper had the same class of defect and was fixed
  by using a wider accumulator with a range check (get_port checks
  tmp_port > 65535).  Apply the same pattern here"

Is that equivalent, though?  get_port() uses a fixed-width u32 and
re-tests after every digit, so it cannot be affected by the width of
unsigned long:

net/netfilter/nf_conntrack_ftp.c:get_port() {
	u32 tmp_port = 0;
	...
		} else if (data[i] >= '0' && data[i] <= '9') {
			tmp_port = tmp_port*10 + data[i] - '0';
			if (tmp_port > 65535)
				break;
	...
}

Would a width-independent form work better here, for example parsing
with simple_strtoull() into an unsigned long long, using kstrtou16() and
checking for -ERANGE, or the ftp-style per-digit bound?

>  	*ad_end_p = data;
>  
>  	return 0;

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/AS8P251MB0001B81958B28F762352EF3DC8B32%40AS8P251MB0001.EURP251.PROD.OUTLOOK.COM

Re: [PATCH] netfilter: conntrack_irc: fix port value truncation in parse_dcc()

[Aamir Ahmed]

On Wed, Sep 09, 2026 at 09:40:13PM +0000, Sashiko AI review wrote:
> Does the truncation still happen before the new check runs on ILP32
> (i386, ARM32, MIPS32), where unsigned long is 32 bits?

Yes, it does.

Florian mentioned on the amanda thread a queued series which adds a
shared parser for all the helpers:

  https://patchwork.ozlabs.org/project/netfilter-devel/patch/20260503083220.630655-2-rc@rexion.ai/

That should cover this. I will drop my patch for now, thanks.

Kind Regards

Aamir A.