netfilter-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Florian Westphal <fw@strlen.de>
To: <netfilter-devel@vger.kernel.org>
Cc: Florian Westphal <fw@strlen.de>
Subject: [PATCH 5/5] netfilter: x_tables: don't move to non-existant next rule
Date: Tue, 22 Mar 2016 18:02:53 +0100	[thread overview]
Message-ID: <1458666173-24318-6-git-send-email-fw@strlen.de> (raw)
In-Reply-To: <1458666173-24318-1-git-send-email-fw@strlen.de>

Ben Hawkes reported an out-of-bounds write in mark_source_chains().
This was caused by improper underflow check -- we should have bailed
earlier.

The underflow check has been fixed in the preceeding change
("netfilter: x_tables: fix unconditional helper").

Just to be safe also add checks to mark_source_chains() in case we have other
bugs that would cause such a condition.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 net/ipv4/netfilter/arp_tables.c | 8 +++++---
 net/ipv4/netfilter/ip_tables.c  | 4 ++++
 net/ipv6/netfilter/ip6_tables.c | 4 ++++
 3 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/net/ipv4/netfilter/arp_tables.c b/net/ipv4/netfilter/arp_tables.c
index a2002ff..13266f4 100644
--- a/net/ipv4/netfilter/arp_tables.c
+++ b/net/ipv4/netfilter/arp_tables.c
@@ -439,6 +439,8 @@ static int mark_source_chains(const struct xt_table_info *newinfo,
 				size = e->next_offset;
 				e = (struct arpt_entry *)
 					(entry0 + pos + size);
+				if (pos + size >= newinfo->size)
+					return 0;
 				e->counters.pcnt = pos;
 				pos += size;
 			} else {
@@ -461,6 +463,8 @@ static int mark_source_chains(const struct xt_table_info *newinfo,
 				} else {
 					/* ... this is a fallthru */
 					newpos = pos + e->next_offset;
+					if (newpos >= newinfo->size)
+						return 0;
 				}
 				e = (struct arpt_entry *)
 					(entry0 + newpos);
@@ -682,10 +686,8 @@ static int translate_table(struct xt_table_info *newinfo, void *entry0,
 		}
 	}
 
-	if (!mark_source_chains(newinfo, repl->valid_hooks, entry0)) {
-		duprintf("Looping hook\n");
+	if (!mark_source_chains(newinfo, repl->valid_hooks, entry0))
 		return -ELOOP;
-	}
 
 	/* Finally, each sanity check must pass */
 	i = 0;
diff --git a/net/ipv4/netfilter/ip_tables.c b/net/ipv4/netfilter/ip_tables.c
index 45b1d97..c4836f0 100644
--- a/net/ipv4/netfilter/ip_tables.c
+++ b/net/ipv4/netfilter/ip_tables.c
@@ -520,6 +520,8 @@ mark_source_chains(const struct xt_table_info *newinfo,
 				size = e->next_offset;
 				e = (struct ipt_entry *)
 					(entry0 + pos + size);
+				if (WARN_ON(pos + size >= newinfo->size))
+					return 0;
 				e->counters.pcnt = pos;
 				pos += size;
 			} else {
@@ -541,6 +543,8 @@ mark_source_chains(const struct xt_table_info *newinfo,
 				} else {
 					/* ... this is a fallthru */
 					newpos = pos + e->next_offset;
+					if (WARN_ON(newpos >= newinfo->size))
+						return 0;
 				}
 				e = (struct ipt_entry *)
 					(entry0 + newpos);
diff --git a/net/ipv6/netfilter/ip6_tables.c b/net/ipv6/netfilter/ip6_tables.c
index 85c0942..ab7cdbf 100644
--- a/net/ipv6/netfilter/ip6_tables.c
+++ b/net/ipv6/netfilter/ip6_tables.c
@@ -532,6 +532,8 @@ mark_source_chains(const struct xt_table_info *newinfo,
 				size = e->next_offset;
 				e = (struct ip6t_entry *)
 					(entry0 + pos + size);
+				if (pos + size >= newinfo->size)
+					return 0;
 				e->counters.pcnt = pos;
 				pos += size;
 			} else {
@@ -553,6 +555,8 @@ mark_source_chains(const struct xt_table_info *newinfo,
 				} else {
 					/* ... this is a fallthru */
 					newpos = pos + e->next_offset;
+					if (newpos >= newinfo->size)
+						return 0;
 				}
 				e = (struct ip6t_entry *)
 					(entry0 + newpos);
-- 
2.4.10


  parent reply	other threads:[~2016-03-22 17:02 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-03-22 17:02 [PATCH nf v3] netfilter: x_tables: perform more sanity tests on rule set Florian Westphal
2016-03-22 17:02 ` [PATCH 1/5] netfilter: x_tables: validate e->target_offset early Florian Westphal
2016-03-24 20:17   ` Pablo Neira Ayuso
2016-03-22 17:02 ` [PATCH 2/5] netfilter: x_tables: make sure e->next_offset covers remaining blob size Florian Westphal
2016-03-24 20:18   ` Pablo Neira Ayuso
2016-03-22 17:02 ` [PATCH 3/5] netfilter: x_tables: add and use xt_check_entry_target Florian Westphal
2016-03-24 20:18   ` Pablo Neira Ayuso
2016-03-25 11:45   ` Florian Westphal
2016-03-25 14:27     ` Pablo Neira Ayuso
2016-03-22 17:02 ` [PATCH 4/5] netfilter: x_tables: fix unconditional helper Florian Westphal
2016-03-24 20:18   ` Pablo Neira Ayuso
2016-03-22 17:02 ` Florian Westphal [this message]
2016-03-22 17:22   ` [PATCH 5/5] netfilter: x_tables: don't move to non-existant next rule Pablo Neira Ayuso

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=1458666173-24318-6-git-send-email-fw@strlen.de \
    --to=fw@strlen.de \
    --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;
as well as URLs for NNTP newsgroup(s).