|
|
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
|