Skip to content

Commit

Permalink
netfilter: nf_conntrack_irc: make sure string is terminated before ca…
Browse files Browse the repository at this point in the history
…lling simple_strtoul

Alexey Dobriyan points out:

1. simple_strtoul() silently accepts all characters for given base even
   if result won't fit into unsigned long. This is amazing stupidity in
   itself, but

2. nf_conntrack_irc helper use simple_strtoul() for DCC request parsing.
   Data first copied into 64KB buffer, so theoretically nothing prevents
   reading past the end of it, since data comes from network given 1).

This is not actually a problem currently since we're guaranteed to have
a 0 byte in skb_shared_info or in the buffer the data is copied to, but
to make this more robust, make sure the string is actually terminated.

Signed-off-by: Patrick McHardy <kaber@trash.net>
Signed-off-by: David S. Miller <davem@davemloft.net>
  • Loading branch information
kaber authored and davem330 committed Sep 8, 2008
1 parent 51807e9 commit e3b802b
Showing 1 changed file with 10 additions and 0 deletions.
10 changes: 10 additions & 0 deletions net/netfilter/nf_conntrack_irc.c
Original file line number Diff line number Diff line change
Expand Up @@ -68,11 +68,21 @@ static const char *const dccprotos[] = {
static int parse_dcc(char *data, const char *data_end, u_int32_t *ip,
u_int16_t *port, char **ad_beg_p, char **ad_end_p)
{
char *tmp;

/* at least 12: "AAAAAAAA P\1\n" */
while (*data++ != ' ')
if (data > data_end - 12)
return -1;

/* 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++)
if (*tmp == '\n')
break;
if (tmp > data_end || *tmp != '\n')
return -1;

*ad_beg_p = data;
*ip = simple_strtoul(data, &data, 10);

Expand Down

0 comments on commit e3b802b

Please sign in to comment.