[PATCH] netfilter: nf_conntrack_irc: fix parse_dcc() off-by-one OOB read

[PATCH] netfilter: nf_conntrack_irc: fix parse_dcc() off-by-one OOB read

[Muhammad Bilal]

parse_dcc() treats data_end as an inclusive end pointer, but its only
caller passes data_limit = ib_ptr + datalen, which points one past the
last valid byte.

The newline search loop iterates while tmp <= data_end, so when no
newline is present, *tmp is read at tmp == data_end, one byte beyond
the region filled by skb_header_pointer().

irc_buffer is kmalloc'd as MAX_SEARCH_SIZE + 1 bytes and datalen is
capped at MAX_SEARCH_SIZE, so the stray read does not fault.  The byte
is uninitialized or stale; if it contains an ASCII digit, simple_strtoul
will consume it and produce a wrong DCC IP or port in the conntrack
expectation.  The extra allocation byte is also a fragile guard: if the
cap or allocation size changes, this becomes a real out-of-bounds read.

Change the loop and its post-loop check to use strict less-than,
consistent with the caller's exclusive-end convention.  Update the
function comment accordingly.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Signed-off-by: Muhammad Bilal <meatuni001@gmail.com>
---
 net/netfilter/nf_conntrack_irc.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/netfilter/nf_conntrack_irc.c b/net/netfilter/nf_conntrack_irc.c
index 522183b9a..9a7b8f622 100644
--- a/net/netfilter/nf_conntrack_irc.c
+++ b/net/netfilter/nf_conntrack_irc.c
@@ -59,7 +59,7 @@ static const char *const dccprotos[] = {
 /* tries to get the ip_addr and port out of a dcc command
  * return value: -1 on failure, 0 on success
  *	data		pointer to first byte of DCC command data
- *	data_end	pointer to last byte of dcc command data
+ *	data_end	one past end of data
  *	ip		returns parsed ip of dcc command
  *	port		returns parsed port of dcc command
  *	ad_beg_p	returns pointer to first byte of addr data
@@ -77,10 +77,10 @@ static int parse_dcc(char *data, const char *data_end, __be32 *ip,
 
 	/* Make sure we have a newline character within the packet boundaries
 	 * because simple_strtoul parses until the first invalid character. */
-	for (tmp = data; tmp <= data_end; tmp++)
+	for (tmp = data; tmp < data_end; tmp++)
 		if (*tmp == '\n')
 			break;
-	if (tmp > data_end || *tmp != '\n')
+	if (tmp >= data_end || *tmp != '\n')
 		return -1;
 
 	*ad_beg_p = data;
-- 
2.54.0

Re: [PATCH] netfilter: nf_conntrack_irc: fix parse_dcc() off-by-one OOB read

[Pablo Neira Ayuso]

On Tue, May 19, 2026 at 05:23:28PM -0400, Muhammad Bilal wrote:
> parse_dcc() treats data_end as an inclusive end pointer, but its only
> caller passes data_limit = ib_ptr + datalen, which points one past the
> last valid byte.
> 
> The newline search loop iterates while tmp <= data_end, so when no
> newline is present, *tmp is read at tmp == data_end, one byte beyond
> the region filled by skb_header_pointer().
> 
> irc_buffer is kmalloc'd as MAX_SEARCH_SIZE + 1 bytes and datalen is
> capped at MAX_SEARCH_SIZE, so the stray read does not fault. 

No crash here.

> The byte is uninitialized or stale; if it contains an ASCII digit,
> simple_strtoul will consume it and produce a wrong DCC IP or port in
> the conntrack expectation.  The extra allocation byte is also a
> fragile guard: if the cap or allocation size changes, this becomes a
> real out-of-bounds read.

Other helpers replaced simple_stroul() already which is probably the
way to go.

This is nf-next material.

> Change the loop and its post-loop check to use strict less-than,
> consistent with the caller's exclusive-end convention.  Update the
> function comment accordingly.
>
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Cc: stable@vger.kernel.org
> Signed-off-by: Muhammad Bilal <meatuni001@gmail.com>
> ---
>  net/netfilter/nf_conntrack_irc.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/net/netfilter/nf_conntrack_irc.c b/net/netfilter/nf_conntrack_irc.c
> index 522183b9a..9a7b8f622 100644
> --- a/net/netfilter/nf_conntrack_irc.c
> +++ b/net/netfilter/nf_conntrack_irc.c
> @@ -59,7 +59,7 @@ static const char *const dccprotos[] = {
>  /* tries to get the ip_addr and port out of a dcc command
>   * return value: -1 on failure, 0 on success
>   *	data		pointer to first byte of DCC command data
> - *	data_end	pointer to last byte of dcc command data
> + *	data_end	one past end of data
>   *	ip		returns parsed ip of dcc command
>   *	port		returns parsed port of dcc command
>   *	ad_beg_p	returns pointer to first byte of addr data
> @@ -77,10 +77,10 @@ static int parse_dcc(char *data, const char *data_end, __be32 *ip,
>  
>  	/* Make sure we have a newline character within the packet boundaries
>  	 * because simple_strtoul parses until the first invalid character. */
> -	for (tmp = data; tmp <= data_end; tmp++)
> +	for (tmp = data; tmp < data_end; tmp++)
>  		if (*tmp == '\n')
>  			break;
> -	if (tmp > data_end || *tmp != '\n')
> +	if (tmp >= data_end || *tmp != '\n')
>  		return -1;
>  
>  	*ad_beg_p = data;
> -- 
> 2.54.0
> 

Re: [PATCH] netfilter: nf_conntrack_irc: fix parse_dcc() off-by-one OOB read

[Muhammad Bilal]

On Tue, May 19, 2026 at 05:29:54PM +0200, Pablo Neira Ayuso wrote:
> Other helpers replaced simple_stroul() already which is probably the
> way to go.

I can send a follow-up patch replacing simple_strtoul() in parse_dcc()
once this lands in nf-next, if that works for you.

> This is nf-next material.

Understood.