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: stable@vger.kernel.org
Signed-off-by: Muhammad Bilal <meatuni001@gmail.com>
Signed-off-by: Florian Westphal <fw@strlen.de>

diff --git a/net/netfilter/nf_conntrack_irc.c b/net/netfilter/nf_conntrack_irc.c
index 522183b9a60465884a82f24429ac18e4fcf541b3..9a7b8f6221eb0dbb7ccaa4670e565cdf3fa9a56b 100644 (file)
--- 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;