CVE-2016-3134 netfilter: missing bounds check in ipt_entry struct (rhbz 1317383 1317384)

This commit is contained in:
Josh Boyer 2016-03-14 08:50:20 -04:00
parent 835b170e9c
commit b653e2ee31
2 changed files with 154 additions and 0 deletions

View File

@ -619,6 +619,9 @@ Patch663: USB-serial-ftdi_sio-Add-support-for-ICP-DAS-I-756xU-.patch
#CVE-2016-3135 rhbz 1317386 1317387 #CVE-2016-3135 rhbz 1317386 1317387
Patch664: netfilter-x_tables-check-for-size-overflow.patch Patch664: netfilter-x_tables-check-for-size-overflow.patch
#CVE-2016-3134 rhbz 1317383 1317384
Patch665: netfilter-x_tables-deal-with-bogus-nextoffset-values.patch
# END OF PATCH DEFINITIONS # END OF PATCH DEFINITIONS
%endif %endif
@ -2141,6 +2144,7 @@ fi
# #
%changelog %changelog
* Mon Mar 14 2016 Josh Boyer <jwboyer@fedoraproject.org> * Mon Mar 14 2016 Josh Boyer <jwboyer@fedoraproject.org>
- CVE-2016-3134 netfilter: missing bounds check in ipt_entry struct (rhbz 1317383 1317384)
- CVE-2016-3135 netfilter: size overflow in x_tables (rhbz 1317386 1317387) - CVE-2016-3135 netfilter: size overflow in x_tables (rhbz 1317386 1317387)
* Fri Mar 11 2016 Josh Boyer <jwboyer@fedoraproject.org> * Fri Mar 11 2016 Josh Boyer <jwboyer@fedoraproject.org>

View File

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