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

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 [email protected]

Signed-off-by Muhammad Bilal <[email protected]>

---
 net/netfilter/nf_conntrack_irc.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

net/netfilter/nf_conntrack_irc.c +3 -3

index 522183b9a..9a7b8f622 100644
@@ -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;

Signature · 1 line

2.54.0

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

This is nf-next material.