netfilter: x_tables: don't move to non-existent next rule

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.

Base chains enforce absolute verdict.

User defined chains are supposed to end with an unconditional return,
xtables userspace adds them automatically.

But if such return is missing we will move to non-existent next rule.

Reported-by: Ben Hawkes <hawkes@google.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>

diff --git a/net/ipv4/netfilter/arp_tables.c b/net/ipv4/netfilter/arp_tables.c
index 4133b0f513afe5a40e523511f24b4652c68b1b57..82a434bf865366e1605f855697f17b291e585ad4 100644 (file)
--- 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);
@@ -691,10 +695,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 631c100a13384203c06ec531ef901cd3aac74f90..e301a3db4717ab8d6e161525ba8350343d64103a 100644 (file)
--- 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 (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 (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 86b67b70b62677ac014577f58d0bdafdd18ecf9e..7b3335bce3fda4c719625fc04e0783b0fd4448f7 100644 (file)
--- 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);