[PATCH] netfilter: conntrack_amanda: fix port value truncation

amanda_help() uses simple_strtoul() to parse the port number from
Amanda CONNECT replies, passing the result directly through htons()
into a __be16.  simple_strtoul() returns unsigned long, so values
above 65535 are silently truncated by the implicit conversion to u16
inside htons().

The existing check "port == 0 || len > 5" is insufficient: it
catches values that truncate to zero (e.g. 65536) and strings longer
than 5 digits, but misses values 65537-99999 whose u16 truncation is
non-zero.  For example, port 65537 becomes port 1, creating a
conntrack expectation for an unintended port.

Parse into an unsigned long and explicitly reject values above 65535
before the htons() conversion, mirroring the pattern used by the FTP
helper's get_port() and the recent IPVS FTP fix (commit
e625a9477d12).

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Aamir Ahmed <redacted>
---
 net/netfilter/nf_conntrack_amanda.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/net/netfilter/nf_conntrack_amanda.c b/net/netfilter/nf_conntrack_amanda.c
index 14ae660491f3..057cef7e2a7e 100644
--- a/net/netfilter/nf_conntrack_amanda.c
+++ b/net/netfilter/nf_conntrack_amanda.c

@@ -91,6 +91,7 @@ static int amanda_help(struct sk_buff *skb,
 	char pbuf[sizeof("65535")], *tmp;
 	u16 len;
 	__be16 port;
+	unsigned long tmp_port;
 	int ret = NF_ACCEPT;
 	nf_nat_amanda_hook_fn *nf_nat_amanda;
 

@@ -132,10 +133,11 @@ static int amanda_help(struct sk_buff *skb,
 			break;
 		pbuf[len] = '\0';
 
-		port = htons(simple_strtoul(pbuf, &tmp, 10));
+		tmp_port = simple_strtoul(pbuf, &tmp, 10);
 		len = tmp - pbuf;
-		if (port == 0 || len > 5)
+		if (tmp_port == 0 || tmp_port > 65535 || len > 5)
 			break;
+		port = htons(tmp_port);
 
 		exp = nf_ct_expect_alloc(ct);
 		if (exp == NULL) {

-- 
2.43.0

在 2026/9/7 07:37, Aamir Ahmed 写道:

len = min_t(unsigned int, sizeof(pbuf) - 1, stop - off) already
limits the length so that it cannot be greater than 5. Could the
len > 5 check here be dropped?

The subject is "[PATCH] ...". Since this is a bugfix, please use "[PATCH net] ...".

Reviewed-by: Chenguang Zhao <redacted>

Thanks

Chenguang

Aamir Ahmed [off-list ref] wrote:

amanda_help() uses simple_strtoul() to parse the port number from
Amanda CONNECT replies, passing the result directly through htons()
into a __be16.  simple_strtoul() returns unsigned long, so values
above 65535 are silently truncated by the implicit conversion to u16
inside htons().

The existing check "port == 0 || len > 5" is insufficient: it
catches values that truncate to zero (e.g. 65536) and strings longer
than 5 digits, but misses values 65537-99999 whose u16 truncation is
non-zero.  For example, port 65537 becomes port 1, creating a
conntrack expectation for an unintended port.

Parse into an unsigned long and explicitly reject values above 65535
before the htons() conversion, mirroring the pattern used by the FTP
helper's get_port() and the recent IPVS FTP fix (commit
e625a9477d12).

Is amanda still a thing?  Maybe time to retire this helper?

On Mon, Sep 07, 2026 at 02:56:13PM +0800, Chenguang Zhao wrote:

在 2026/9/7 07:37, Aamir Ahmed 写道:

quoted

amanda_help() uses simple_strtoul() to parse the port number from
Amanda CONNECT replies, passing the result directly through htons()
into a __be16.  simple_strtoul() returns unsigned long, so values
above 65535 are silently truncated by the implicit conversion to u16
inside htons().

The existing check "port == 0 || len > 5" is insufficient: it
catches values that truncate to zero (e.g. 65536) and strings longer
than 5 digits, but misses values 65537-99999 whose u16 truncation is
non-zero.  For example, port 65537 becomes port 1, creating a
conntrack expectation for an unintended port.

Parse into an unsigned long and explicitly reject values above 65535
before the htons() conversion, mirroring the pattern used by the FTP
helper's get_port() and the recent IPVS FTP fix (commit
e625a9477d12).

Please, specify target tree. This must go to nf-next.

On Mon, Sep 07, 2026 at 02:56:13PM +0800, Chenguang Zhao wrote:

len = min_t(unsigned int, sizeof(pbuf) - 1, stop - off) already
limits the length so that it cannot be greater than 5. Could the
len > 5 check here be dropped?

Yes, that is correct actually - will drop it in v2. I will carry your
Reviewed-by on v2 if that is OK with the change.

On Mon, Sep 07, 2026 at 06:37:43PM +0200, Pablo Neira Ayuso wrote:

Please, specify target tree. This must go to nf-next.

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

Kind Regards

Aamir A.