Re: [PATCH net-next 2/2] netfilter: nf_conntrack_amanda: reject port values above 65535

Pablo Neira Ayuso
2026-04-30T16:21:28Z

Hi,

On Thu, Apr 30, 2026 at 09:45:15PM +0530, HACKE-RC wrote:
> amanda_help() converts the result of simple_strtoul() to __be16 via
> htons() without checking the parsed value fits in 16 bits. The
> existing len > 5 guard limits strings to five digits, capping the
> parseable range at 99999, but values 65536-99999 still silently
> truncate on the htons() conversion.
>
> Use an intermediate unsigned long and reject out-of-range values
> before converting to network byte order.
>
> Fixes: 16958900578b ("[NETFILTER]: nf_conntrack/nf_nat: add amanda helper port")
> Signed-off-by: HACKE-RC <rc@rexion.ai>
> ---
> net/netfilter/nf_conntrack_amanda.c | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/net/netfilter/nf_conntrack_amanda.c b/net/netfilter/nf_conntrack_amanda.c
> index d2c09e8dd..58d6c9f29 100644
> --- a/net/netfilter/nf_conntrack_amanda.c
> +++ b/net/netfilter/nf_conntrack_amanda.c
> @@ -88,11 +88,12 @@ static int amanda_help(struct sk_buff *skb,
> struct nf_conntrack_expect *exp;
> struct nf_conntrack_tuple *tuple;
> unsigned int dataoff, start, stop, off, i;
> + nf_nat_amanda_hook_fn *nf_nat_amanda;
> char pbuf[sizeof("65535")], *tmp;
> + unsigned long parsed_port;
> + int ret = NF_ACCEPT;
> u_int16_t len;
> __be16 port;
> - int ret = NF_ACCEPT;
> - nf_nat_amanda_hook_fn *nf_nat_amanda;
>
> /* Only look at packets from the Amanda server */
> if (CTINFO2DIR(ctinfo) == IP_CT_DIR_ORIGINAL)
> @@ -132,10 +133,11 @@ static int amanda_help(struct sk_buff *skb,
> break;
> pbuf[len] = '\0';
>
> - port = htons(simple_strtoul(pbuf, &tmp, 10));
> + parsed_port = simple_strtoul(pbuf, &tmp, 10);

While being here, I would replace this simple_strtoul by a parser
which does not rely on nul-terminated strings.

A similar patch went in for the sip helper recently, maybe you can
just take such function to parse ports, move it to the
nf_conntrack_helper core so it can be shared by helpers.

> len = tmp - pbuf;
> - if (port == 0 || len > 5)
> + if (parsed_port == 0 || parsed_port > 65535 || len > 5)
> break;
> + port = htons(parsed_port);
>
> exp = nf_ct_expect_alloc(ct);
> if (exp == NULL) {
> --
> 2.54.0
>