Re: [PATCH 1/2] netfilter fix u16 overflow in get_port()

[Pablo Neira Ayuso <pablo@netfilter.org>]

Patch subject should be:

  [PATCH nf-next] netfilter: nf_conntrack_ftp: fix u16 overflow in get_port()

On Fri, Apr 10, 2026 at 09:57:33AM -0400, Cyber-JA wrote:
> From: Giuseppe Caruso <giuseppecaruso0990@gmail.com>
> 
> try_number() parses comma-separated decimal values from FTP PORT and
> EPRT commands into a u_int32_t array, but does not validate that each
> value fits in a single octet. RFC 959 specifies that PORT parameters
> are decimal integers in the range 0-255, representing the four octets
> of an IP address followed by two octets encoding the port number.
>
> Values exceeding 255 are silently accepted. In try_rfc959(), the raw
> u32 values are combined via shift-and-OR to form the IP and port:
> 
>   cmd->u3.ip = htonl((array[0] << 24) | (array[1] << 16) |
>                      (array[2] << 8) | array[3]);
>   cmd->u.tcp.port = htons((array[4] << 8) | array[5]);
> 
> When array elements exceed 255, bits from one field bleed into adjacent
> fields after shifting, producing IP addresses and port numbers that
> differ from what the text representation suggests. For example,
> "PORT 10,0,1,2,256,22" yields port (256<<8)|22 = 65558, truncated to
> u16 = 22. This mismatch between the textual and computed values can
> confuse network monitoring tools that parse FTP commands independently.

Fair enough. But stricter parser is better, of course.

> Reject the command by returning 0 (no match) when any accumulated
> value exceeds 255.

This can probably be expanded to say that "returning 0 (no match)
results in no expectation is being created".

Nothing is really "rejected" (that happens by returning -1), no
packets are dropped, just to clarify.

> Signed-off-by: Giuseppe Caruso <giuseppecaruso0990@gmail.com>
> ---
>  net/netfilter/nf_conntrack_ftp.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/net/netfilter/nf_conntrack_ftp.c b/net/netfilter/nf_conntrack_ftp.c
> index 5e00f9123c38..680dd7560ebc 100644
> --- a/net/netfilter/nf_conntrack_ftp.c
> +++ b/net/netfilter/nf_conntrack_ftp.c
> @@ -195,7 +195,7 @@ static int try_rfc1123(const char *data, size_t dlen,
>  static int get_port(const char *data, int start, size_t dlen, char delim,
>  		    __be16 *port)
>  {
> -	u_int16_t tmp_port = 0;
> +	u_int32_t tmp_port = 0;
>  	int i;
>  
>  	for (i = start; i < dlen; i++) {
> @@ -207,8 +207,14 @@ static int get_port(const char *data, int start, size_t dlen, char delim,
>  			pr_debug("get_port: return %d\n", tmp_port);
>  			return i + 1;
>  		}
> -		else if (data[i] >= '0' && data[i] <= '9')
> +		else if (data[i] >= '0' && data[i] <= '9'){
>  			tmp_port = tmp_port*10 + data[i] - '0';
> +			if (tmp_port > 65535) {
> +				pr_debug("get_port: port %u out of range.\n",
> +					 tmp_port);
> +				break;
> +			}
> +		}
>  		else { /* Some other crap */
>  			pr_debug("get_port: invalid char.\n");
>  			break;
> -- 
> 2.53.0
> 
>