b653e2e
Subject:    [PATCH nf] netfilter: x_tables: deal with bogus nextoffset values
b653e2e
From:       Florian Westphal <fw () strlen ! de>
b653e2e
Date:       2016-03-10 0:56:02
b653e2e
b653e2e
Ben Hawkes says:
b653e2e
b653e2e
 In the mark_source_chains function (net/ipv4/netfilter/ip_tables.c) it
b653e2e
 is possible for a user-supplied ipt_entry structure to have a large
b653e2e
 next_offset field. This field is not bounds checked prior to writing a
b653e2e
 counter value at the supplied offset.
b653e2e
b653e2e
Problem is that xt_entry_foreach() macro stops iterating once e->next_offset
b653e2e
is out of bounds, assuming this is the last entry.
b653e2e
b653e2e
With malformed data thats not necessarily the case so we can
b653e2e
write outside of allocated area later as we might not have walked the
b653e2e
entire blob.
b653e2e
b653e2e
Fix this by simplifying mark_source_chains -- it already has to check
b653e2e
if nextoff is in range to catch invalid jumps, so just do the check
b653e2e
when we move to a next entry as well.
b653e2e
b653e2e
Signed-off-by: Florian Westphal <fw@strlen.de>
b653e2e
---
b653e2e
 net/ipv4/netfilter/arp_tables.c | 16 ++++++++--------
b653e2e
 net/ipv4/netfilter/ip_tables.c  | 15 ++++++++-------
b653e2e
 net/ipv6/netfilter/ip6_tables.c | 13 ++++++-------
b653e2e
 3 files changed, 22 insertions(+), 22 deletions(-)
b653e2e
b653e2e
diff --git a/net/ipv4/netfilter/arp_tables.c b/net/ipv4/netfilter/arp_tables.c
b653e2e
index b488cac..5a0b591 100644
b653e2e
--- a/net/ipv4/netfilter/arp_tables.c
b653e2e
+++ b/net/ipv4/netfilter/arp_tables.c
b653e2e
@@ -437,6 +437,10 @@ static int mark_source_chains(const struct xt_table_info *newinfo,
b653e2e
 
b653e2e
 				/* Move along one */
b653e2e
 				size = e->next_offset;
b653e2e
+
b653e2e
+				if (pos + size > newinfo->size - sizeof(*e))
b653e2e
+					return 0;
b653e2e
+
b653e2e
 				e = (struct arpt_entry *)
b653e2e
 					(entry0 + pos + size);
b653e2e
 				e->counters.pcnt = pos;
b653e2e
@@ -447,14 +451,6 @@ static int mark_source_chains(const struct xt_table_info *newinfo,
b653e2e
 				if (strcmp(t->target.u.user.name,
b653e2e
 					   XT_STANDARD_TARGET) == 0 &&
b653e2e
 				    newpos >= 0) {
b653e2e
-					if (newpos > newinfo->size -
b653e2e
-						sizeof(struct arpt_entry)) {
b653e2e
-						duprintf("mark_source_chains: "
b653e2e
-							"bad verdict (%i)\n",
b653e2e
-								newpos);
b653e2e
-						return 0;
b653e2e
-					}
b653e2e
-
b653e2e
 					/* This a jump; chase it. */
b653e2e
 					duprintf("Jump rule %u -> %u\n",
b653e2e
 						 pos, newpos);
b653e2e
@@ -462,6 +458,10 @@ static int mark_source_chains(const struct xt_table_info *newinfo,
b653e2e
 					/* ... this is a fallthru */
b653e2e
 					newpos = pos + e->next_offset;
b653e2e
 				}
b653e2e
+
b653e2e
+				if (newpos > newinfo->size - sizeof(*e))
b653e2e
+					return 0;
b653e2e
+
b653e2e
 				e = (struct arpt_entry *)
b653e2e
 					(entry0 + newpos);
b653e2e
 				e->counters.pcnt = pos;
b653e2e
diff --git a/net/ipv4/netfilter/ip_tables.c b/net/ipv4/netfilter/ip_tables.c
b653e2e
index b99affa..ceb995f 100644
b653e2e
--- a/net/ipv4/netfilter/ip_tables.c
b653e2e
+++ b/net/ipv4/netfilter/ip_tables.c
b653e2e
@@ -519,6 +519,10 @@ mark_source_chains(const struct xt_table_info *newinfo,
b653e2e
 
b653e2e
 				/* Move along one */
b653e2e
 				size = e->next_offset;
b653e2e
+
b653e2e
+				if (pos + size > newinfo->size - sizeof(*e))
b653e2e
+					return 0;
b653e2e
+
b653e2e
 				e = (struct ipt_entry *)
b653e2e
 					(entry0 + pos + size);
b653e2e
 				e->counters.pcnt = pos;
b653e2e
@@ -529,13 +533,6 @@ mark_source_chains(const struct xt_table_info *newinfo,
b653e2e
 				if (strcmp(t->target.u.user.name,
b653e2e
 					   XT_STANDARD_TARGET) == 0 &&
b653e2e
 				    newpos >= 0) {
b653e2e
-					if (newpos > newinfo->size -
b653e2e
-						sizeof(struct ipt_entry)) {
b653e2e
-						duprintf("mark_source_chains: "
b653e2e
-							"bad verdict (%i)\n",
b653e2e
-								newpos);
b653e2e
-						return 0;
b653e2e
-					}
b653e2e
 					/* This a jump; chase it. */
b653e2e
 					duprintf("Jump rule %u -> %u\n",
b653e2e
 						 pos, newpos);
b653e2e
@@ -543,6 +540,10 @@ mark_source_chains(const struct xt_table_info *newinfo,
b653e2e
 					/* ... this is a fallthru */
b653e2e
 					newpos = pos + e->next_offset;
b653e2e
 				}
b653e2e
+
b653e2e
+				if (newpos > newinfo->size - sizeof(*e))
b653e2e
+					return 0;
b653e2e
+
b653e2e
 				e = (struct ipt_entry *)
b653e2e
 					(entry0 + newpos);
b653e2e
 				e->counters.pcnt = pos;
b653e2e
diff --git a/net/ipv6/netfilter/ip6_tables.c b/net/ipv6/netfilter/ip6_tables.c
b653e2e
index 99425cf..d88a794 100644
b653e2e
--- a/net/ipv6/netfilter/ip6_tables.c
b653e2e
+++ b/net/ipv6/netfilter/ip6_tables.c
b653e2e
@@ -531,6 +531,8 @@ mark_source_chains(const struct xt_table_info *newinfo,
b653e2e
 
b653e2e
 				/* Move along one */
b653e2e
 				size = e->next_offset;
b653e2e
+				if (pos + size > newinfo->size - sizeof(*e))
b653e2e
+					return 0;
b653e2e
 				e = (struct ip6t_entry *)
b653e2e
 					(entry0 + pos + size);
b653e2e
 				e->counters.pcnt = pos;
b653e2e
@@ -541,13 +543,6 @@ mark_source_chains(const struct xt_table_info *newinfo,
b653e2e
 				if (strcmp(t->target.u.user.name,
b653e2e
 					   XT_STANDARD_TARGET) == 0 &&
b653e2e
 				    newpos >= 0) {
b653e2e
-					if (newpos > newinfo->size -
b653e2e
-						sizeof(struct ip6t_entry)) {
b653e2e
-						duprintf("mark_source_chains: "
b653e2e
-							"bad verdict (%i)\n",
b653e2e
-								newpos);
b653e2e
-						return 0;
b653e2e
-					}
b653e2e
 					/* This a jump; chase it. */
b653e2e
 					duprintf("Jump rule %u -> %u\n",
b653e2e
 						 pos, newpos);
b653e2e
@@ -555,6 +550,10 @@ mark_source_chains(const struct xt_table_info *newinfo,
b653e2e
 					/* ... this is a fallthru */
b653e2e
 					newpos = pos + e->next_offset;
b653e2e
 				}
b653e2e
+
b653e2e
+				if (newpos > newinfo->size - sizeof(*e))
b653e2e
+					return 0;
b653e2e
+
b653e2e
 				e = (struct ip6t_entry *)
b653e2e
 					(entry0 + newpos);
b653e2e
 				e->counters.pcnt = pos;
b653e2e
-- 
b653e2e
2.4.10