netfilter: x_tables: check standard verdicts in core

Userspace must provide a valid verdict to the standard target.

The verdict can be either a jump (signed int > 0), or a return code.

Allowed return codes are either RETURN (pop from stack), NF_ACCEPT, DROP
and QUEUE (latter is allowed for legacy reasons).

Jump offsets (verdict > 0) are checked in more detail later on when
loop-detection is performed.

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 a0c7ce7..c9ffa88 100644 (file)
--- a/net/ipv4/netfilter/arp_tables.c
+++ b/net/ipv4/netfilter/arp_tables.c
@@ -334,11 +334,6 @@ static int mark_source_chains(const struct xt_table_info *newinfo,
                             t->verdict < 0) || visited) {
                                unsigned int oldpos, size;
 
-                               if ((strcmp(t->target.u.user.name,
-                                           XT_STANDARD_TARGET) == 0) &&
-                                   t->verdict < -NF_MAX_VERDICT - 1)
-                                       return 0;
-
                                /* Return: backtrack through the last
                                 * big jump.
                                 */

diff --git a/net/ipv4/netfilter/ip_tables.c b/net/ipv4/netfilter/ip_tables.c
index 4f7153e..c9b57a6 100644 (file)
--- a/net/ipv4/netfilter/ip_tables.c
+++ b/net/ipv4/netfilter/ip_tables.c
@@ -402,11 +402,6 @@ mark_source_chains(const struct xt_table_info *newinfo,
                             t->verdict < 0) || visited) {
                                unsigned int oldpos, size;
 
-                               if ((strcmp(t->target.u.user.name,
-                                           XT_STANDARD_TARGET) == 0) &&
-                                   t->verdict < -NF_MAX_VERDICT - 1)
-                                       return 0;
-
                                /* Return: backtrack through the last
                                   big jump. */
                                do {

diff --git a/net/ipv6/netfilter/ip6_tables.c b/net/ipv6/netfilter/ip6_tables.c
index 6c44033..f469542 100644 (file)
--- a/net/ipv6/netfilter/ip6_tables.c
+++ b/net/ipv6/netfilter/ip6_tables.c
@@ -420,11 +420,6 @@ mark_source_chains(const struct xt_table_info *newinfo,
                             t->verdict < 0) || visited) {
                                unsigned int oldpos, size;
 
-                               if ((strcmp(t->target.u.user.name,
-                                           XT_STANDARD_TARGET) == 0) &&
-                                   t->verdict < -NF_MAX_VERDICT - 1)
-                                       return 0;
-
                                /* Return: backtrack through the last
                                   big jump. */
                                do {

diff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c
index d9deebe..2e4d423 100644 (file)
--- a/net/netfilter/x_tables.c
+++ b/net/netfilter/x_tables.c
@@ -654,6 +654,31 @@ struct compat_xt_standard_target {
        compat_uint_t verdict;
 };
 
+static bool verdict_ok(int verdict)
+{
+       if (verdict > 0)
+               return true;
+
+       if (verdict < 0) {
+               int v = -verdict - 1;
+
+               if (verdict == XT_RETURN)
+                       return true;
+
+               switch (v) {
+               case NF_ACCEPT: return true;
+               case NF_DROP: return true;
+               case NF_QUEUE: return true;
+               default:
+                       break;
+               }
+
+               return false;
+       }
+
+       return false;
+}
+
 int xt_compat_check_entry_offsets(const void *base, const char *elems,
                                  unsigned int target_offset,
                                  unsigned int next_offset)
@@ -675,9 +700,15 @@ int xt_compat_check_entry_offsets(const void *base, const char *elems,
        if (target_offset + t->u.target_size > next_offset)
                return -EINVAL;
 
-       if (strcmp(t->u.user.name, XT_STANDARD_TARGET) == 0 &&
-           COMPAT_XT_ALIGN(target_offset + sizeof(struct compat_xt_standard_target)) != next_offset)
-               return -EINVAL;
+       if (strcmp(t->u.user.name, XT_STANDARD_TARGET) == 0) {
+               const struct compat_xt_standard_target *st = (const void *)t;
+
+               if (COMPAT_XT_ALIGN(target_offset + sizeof(*st)) != next_offset)
+                       return -EINVAL;
+
+               if (!verdict_ok(st->verdict))
+                       return -EINVAL;
+       }
 
        /* compat_xt_entry match has less strict alignment requirements,
         * otherwise they are identical.  In case of padding differences
@@ -757,9 +788,15 @@ int xt_check_entry_offsets(const void *base,
        if (target_offset + t->u.target_size > next_offset)
                return -EINVAL;
 
-       if (strcmp(t->u.user.name, XT_STANDARD_TARGET) == 0 &&
-           XT_ALIGN(target_offset + sizeof(struct xt_standard_target)) != next_offset)
-               return -EINVAL;
+       if (strcmp(t->u.user.name, XT_STANDARD_TARGET) == 0) {
+               const struct xt_standard_target *st = (const void *)t;
+
+               if (XT_ALIGN(target_offset + sizeof(*st)) != next_offset)
+                       return -EINVAL;
+
+               if (!verdict_ok(st->verdict))
+                       return -EINVAL;
+       }
 
        return xt_check_entry_match(elems, base + target_offset,
                                    __alignof__(struct xt_entry_match));