Linux Netfilter development
 help / color / mirror / Atom feed
From: Patrick McHardy <kaber@trash.net>
To: davem@davemloft.net
Cc: Patrick McHardy <kaber@trash.net>, netfilter-devel@vger.kernel.org
Subject: [PATCH 04/04]: netfilter: nf_conntrack_irc: make sure string is terminated before calling simple_strtoul
Date: Thu,  4 Sep 2008 16:15:58 +0200 (MEST)	[thread overview]
Message-ID: <20080904141558.6414.18261.sendpatchset@x2.localnet> (raw)
In-Reply-To: <20080904141553.6414.87829.sendpatchset@x2.localnet>

commit c834a27db90c3b871709c2f3b5d02d5ba62c6c48
Author: Patrick McHardy <kaber@trash.net>
Date:   Thu Sep 4 14:22:06 2008 +0200

    netfilter: nf_conntrack_irc: make sure string is terminated before calling 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>

diff --git a/net/netfilter/nf_conntrack_irc.c b/net/netfilter/nf_conntrack_irc.c
index 1b1226d..20633fd 100644
--- a/net/netfilter/nf_conntrack_irc.c
+++ b/net/netfilter/nf_conntrack_irc.c
@@ -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);
 

  parent reply	other threads:[~2008-09-04 14:16 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-09-04 14:15 [PATCH 00/04]: Netfilter fixes Patrick McHardy
2008-09-04 14:15 ` [PATCH 01/04]: netfilter: nf_conntrack_sip: de-static helper pointers Patrick McHardy
2008-09-08  1:19   ` David Miller
2008-09-04 14:15 ` [PATCH 02/04]: netfilter: nf_conntrack_gre: more locking around keymap list Patrick McHardy
2008-09-04 14:15 ` [PATCH 03/04]: netfilter: nf_conntrack_gre: nf_ct_gre_keymap_flush() fixlet Patrick McHardy
2008-09-08  1:20   ` David Miller
2008-09-04 14:15 ` Patrick McHardy [this message]
2008-09-08  1:21   ` [PATCH 04/04]: netfilter: nf_conntrack_irc: make sure string is terminated before calling simple_strtoul David Miller
2008-09-08  1:23 ` [PATCH 00/04]: Netfilter fixes David Miller
2008-09-08 12:44   ` Patrick McHardy

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20080904141558.6414.18261.sendpatchset@x2.localnet \
    --to=kaber@trash.net \
    --cc=davem@davemloft.net \
    --cc=netfilter-devel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox