[NETFILTER]: Fix possible overflow in netfilters do_replace()

netfilter's do_replace() can overflow on addition within SMP_ALIGN()
and/or on multiplication by NR_CPUS, resulting in a buffer overflow on
the copy_from_user().  In practice, the overflow on addition is
triggerable on all systems, whereas the multiplication one might require
much physical memory to be present due to the check above.  Either is
sufficient to overwrite arbitrary amounts of kernel memory.

I really hate adding the same check to all 4 versions of do_replace(),
but the code is duplicate...

Found by Solar Designer during security audit of OpenVZ.org

Signed-Off-By: Kirill Korotaev <dev@openvz.org>
Signed-Off-By: Solar Designer <solar@openwall.com>
Signed-off-by: Patrck McHardy <kaber@trash.net>
Signed-off-by: David S. Miller <davem@davemloft.net>

diff --git a/net/bridge/netfilter/ebtables.c b/net/bridge/netfilter/ebtables.c
index 00729b3604f8b540e507f74e0aabf4cf6e06fa96..cbd4020cc84d6a142c0128496a80de3909b1f58e 100644 (file)
--- a/net/bridge/netfilter/ebtables.c
+++ b/net/bridge/netfilter/ebtables.c
@@ -934,6 +934,13 @@ static int do_replace(void __user *user, unsigned int len)
                BUGPRINT("Entries_size never zero\n");
                return -EINVAL;
        }
+       /* overflow check */
+       if (tmp.nentries >= ((INT_MAX - sizeof(struct ebt_table_info)) / NR_CPUS -
+                       SMP_CACHE_BYTES) / sizeof(struct ebt_counter))
+               return -ENOMEM;
+       if (tmp.num_counters >= INT_MAX / sizeof(struct ebt_counter))
+               return -ENOMEM;
+
        countersize = COUNTER_OFFSET(tmp.nentries) * 
                                        (highest_possible_processor_id()+1);
        newinfo = (struct ebt_table_info *)

diff --git a/net/ipv4/netfilter/arp_tables.c b/net/ipv4/netfilter/arp_tables.c
index afe3d8f8177d7df83df0264e20f3072077ff1317..dd1048be8a0115b4cd08591fbd3b29b19e9bda36 100644 (file)
--- a/net/ipv4/netfilter/arp_tables.c
+++ b/net/ipv4/netfilter/arp_tables.c
@@ -807,6 +807,13 @@ static int do_replace(void __user *user, unsigned int len)
        if (len != sizeof(tmp) + tmp.size)
                return -ENOPROTOOPT;
 
+       /* overflow check */
+       if (tmp.size >= (INT_MAX - sizeof(struct xt_table_info)) / NR_CPUS -
+                       SMP_CACHE_BYTES)
+               return -ENOMEM;
+       if (tmp.num_counters >= INT_MAX / sizeof(struct xt_counters))
+               return -ENOMEM;
+
        newinfo = xt_alloc_table_info(tmp.size);
        if (!newinfo)
                return -ENOMEM;

diff --git a/net/ipv4/netfilter/ip_tables.c b/net/ipv4/netfilter/ip_tables.c
index 2371b2062c2d812468ad62f4fe19d4360e748f41..16f47c675fefd8304d86174f6d4beeebdec6539b 100644 (file)
--- a/net/ipv4/netfilter/ip_tables.c
+++ b/net/ipv4/netfilter/ip_tables.c
@@ -921,6 +921,13 @@ do_replace(void __user *user, unsigned int len)
        if (len != sizeof(tmp) + tmp.size)
                return -ENOPROTOOPT;
 
+       /* overflow check */
+       if (tmp.size >= (INT_MAX - sizeof(struct xt_table_info)) / NR_CPUS -
+                       SMP_CACHE_BYTES)
+               return -ENOMEM;
+       if (tmp.num_counters >= INT_MAX / sizeof(struct xt_counters))
+               return -ENOMEM;
+
        newinfo = xt_alloc_table_info(tmp.size);
        if (!newinfo)
                return -ENOMEM;

diff --git a/net/ipv6/netfilter/ip6_tables.c b/net/ipv6/netfilter/ip6_tables.c
index 847068fd33676cfd3b6bc510be992baf4ab8d06d..74ff56c322f47ee45b3b873e4672d0bc9c13511a 100644 (file)
--- a/net/ipv6/netfilter/ip6_tables.c
+++ b/net/ipv6/netfilter/ip6_tables.c
@@ -978,6 +978,13 @@ do_replace(void __user *user, unsigned int len)
        if (copy_from_user(&tmp, user, sizeof(tmp)) != 0)
                return -EFAULT;
 
+       /* overflow check */
+       if (tmp.size >= (INT_MAX - sizeof(struct xt_table_info)) / NR_CPUS -
+                       SMP_CACHE_BYTES)
+               return -ENOMEM;
+       if (tmp.num_counters >= INT_MAX / sizeof(struct xt_counters))
+               return -ENOMEM;
+
        newinfo = xt_alloc_table_info(tmp.size);
        if (!newinfo)
                return -ENOMEM;