UBUNTU: SAUCE: [nf,v2] netfilter: x_tables: don't rely on well-behaving userspace

BugLink: http://bugs.launchpad.net/bugs/1555338
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.

Also, check that the offset meets the xtables_entry alignment.

Reported-by: Ben Hawkes <hawkes@google.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Chris J. Arges <chris.j.arges@canonical.com>
Acked-by: Brad Figg <brad.figg@canonical.com>
Signed-off-by: Brad Figg <brad.figg@canonical.com>
Signed-off-by: Tim Gardner <tim.gardner@canonical.com>

diff --git a/net/ipv4/netfilter/arp_tables.c b/net/ipv4/netfilter/arp_tables.c
index 9e9d9afd18f745f810dc5d985c5c4532ef8053a3..b1fedb334bc145fb0560d2f6d1ce97273358a1b3 100644 (file)
--- a/net/ipv4/netfilter/arp_tables.c
+++ b/net/ipv4/netfilter/arp_tables.c
@@ -295,6 +295,17 @@ static inline bool unconditional(const struct arpt_entry *e)
               memcmp(&e->arp, &uncond, sizeof(uncond)) == 0;
 }
 
+static bool next_offset_ok(const struct xt_table_info *t, unsigned int newpos)
+{
+       if (newpos > t->size - sizeof(struct arpt_entry))
+               return false;
+
+       if (newpos % __alignof__(struct arpt_entry) != 0)
+               return false;
+
+       return true;
+}
+
 /* Figures out from what hook each rule can be called: returns 0 if
  * there are loops.  Puts hook bitmask in comefrom.
  */
@@ -358,6 +369,8 @@ static int mark_source_chains(const struct xt_table_info *newinfo,
 
                                /* Move along one */
                                size = e->next_offset;
+                               if (!next_offset_ok(newinfo, pos + size))
+                                       return 0;
                                e = entry0 + pos + size;
                                if (pos + size >= newinfo->size)
                                        return 0;
@@ -380,6 +393,10 @@ static int mark_source_chains(const struct xt_table_info *newinfo,
                                        if (newpos >= newinfo->size)
                                                return 0;
                                }
+
+                               if (!next_offset_ok(newinfo, newpos))
+                                       return 0;
+
                                e = entry0 + newpos;
                                e->counters.pcnt = pos;
                                pos = newpos;

diff --git a/net/ipv4/netfilter/ip_tables.c b/net/ipv4/netfilter/ip_tables.c
index 622ed2887cd563dc5e708028d2f17726c8ca1c29..964226b48bc2cf279bf10027777fd74f300aa16d 100644 (file)
--- a/net/ipv4/netfilter/ip_tables.c
+++ b/net/ipv4/netfilter/ip_tables.c
@@ -370,6 +370,17 @@ ipt_do_table(struct sk_buff *skb,
        else return verdict;
 }
 
+static bool next_offset_ok(const struct xt_table_info *t, unsigned int newpos)
+{
+       if (newpos > t->size - sizeof(struct ipt_entry))
+               return false;
+
+       if (newpos % __alignof__(struct ipt_entry) != 0)
+               return false;
+
+       return true;
+}
+
 /* Figures out from what hook each rule can be called: returns 0 if
    there are loops.  Puts hook bitmask in comefrom. */
 static int
@@ -430,6 +441,8 @@ mark_source_chains(const struct xt_table_info *newinfo,
 
                                /* Move along one */
                                size = e->next_offset;
+                               if (!next_offset_ok(newinfo, pos + size))
+                                       return 0;
                                e = entry0 + pos + size;
                                if (pos + size >= newinfo->size)
                                        return 0;
@@ -452,6 +465,10 @@ mark_source_chains(const struct xt_table_info *newinfo,
                                        if (newpos >= newinfo->size)
                                                return 0;
                                }
+
+                               if (!next_offset_ok(newinfo, newpos))
+                                       return 0;
+
                                e = entry0 + newpos;
                                e->counters.pcnt = pos;
                                pos = newpos;

diff --git a/net/ipv6/netfilter/ip6_tables.c b/net/ipv6/netfilter/ip6_tables.c
index 1f90644056ac784f1a455751796d5fa0d9441915..5b422a7d9bb66e56b3eb48ab29d86a8b59b7f551 100644 (file)
--- a/net/ipv6/netfilter/ip6_tables.c
+++ b/net/ipv6/netfilter/ip6_tables.c
@@ -389,6 +389,17 @@ ip6t_do_table(struct sk_buff *skb,
        else return verdict;
 }
 
+static bool next_offset_ok(const struct xt_table_info *t, unsigned int newpos)
+{
+       if (newpos > t->size - sizeof(struct ip6t_entry))
+               return false;
+
+       if (newpos % __alignof__(struct ip6t_entry) != 0)
+               return false;
+
+       return true;
+}
+
 /* Figures out from what hook each rule can be called: returns 0 if
    there are loops.  Puts hook bitmask in comefrom. */
 static int
@@ -449,6 +460,8 @@ mark_source_chains(const struct xt_table_info *newinfo,
 
                                /* Move along one */
                                size = e->next_offset;
+                               if (!next_offset_ok(newinfo, pos + size))
+                                       return 0;
                                e = entry0 + pos + size;
                                if (pos + size >= newinfo->size)
                                        return 0;
@@ -471,6 +484,10 @@ mark_source_chains(const struct xt_table_info *newinfo,
                                        if (newpos >= newinfo->size)
                                                return 0;
                                }
+
+                               if (!next_offset_ok(newinfo, newpos))
+                                       return 0;
+
                                e = entry0 + newpos;
                                e->counters.pcnt = pos;
                                pos = newpos;