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

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

[Cyber-JA]

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.

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

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

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

[Pablo Neira Ayuso]

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
> 
>