iptables: improve chain name validation

As pointed out by Andrew Domaszek, iptables allows whitespace to be included in
chain names.  This causes issues with iptables-restore, and later iptables
actions on the chain.  Attached patch disallows whitespace, and also consolidates
all chain name checking into a new function.

This closes netfilter bugzilla #855.

[ Included ip6tables changed as well --pablo ]

Signed-off-by: Phil Oester <kernel@linuxace.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>

diff --git a/iptables/ip6tables.c b/iptables/ip6tables.c
index 76de36726144acbdc4cb1944acc2a6033a9ab3bc..a5199d5ef035d0cbc2d8fa9ec385c068af26c8c9 100644 (file)
--- a/iptables/ip6tables.c
+++ b/iptables/ip6tables.c
@@ -387,6 +387,32 @@ parse_rulenumber(const char *rule)
        return rulenum;
 }
 
+static void
+parse_chain(const char *chainname)
+{
+       const char *ptr;
+
+       if (strlen(chainname) >= XT_EXTENSION_MAXNAMELEN)
+               xtables_error(PARAMETER_PROBLEM,
+                          "chain name `%s' too long (must be under %u chars)",
+                          chainname, XT_EXTENSION_MAXNAMELEN);
+
+       if (*chainname == '-' || *chainname == '!')
+               xtables_error(PARAMETER_PROBLEM,
+                          "chain name not allowed to start "
+                          "with `%c'\n", *chainname);
+
+       if (xtables_find_target(chainname, XTF_TRY_LOAD))
+               xtables_error(PARAMETER_PROBLEM,
+                          "chain name may not clash "
+                          "with target name\n");
+
+       for (ptr = chainname; *ptr; ptr++)
+               if (isspace(*ptr))
+                       xtables_error(PARAMETER_PROBLEM,
+                                  "Invalid chain name `%s'", chainname);
+}
+
 static const char *
 parse_target(const char *targetname)
 {
@@ -1432,14 +1458,7 @@ int do_command6(int argc, char *argv[], char **table,
                        break;
 
                case 'N':
-                       if (optarg && (*optarg == '-' || *optarg == '!'))
-                               xtables_error(PARAMETER_PROBLEM,
-                                          "chain name not allowed to start "
-                                          "with `%c'\n", *optarg);
-                       if (xtables_find_target(optarg, XTF_TRY_LOAD))
-                               xtables_error(PARAMETER_PROBLEM,
-                                          "chain name may not clash "
-                                          "with target name\n");
+                       parse_chain(optarg);
                        add_command(&command, CMD_NEW_CHAIN, CMD_NONE,
                                    cs.invert);
                        chain = optarg;
@@ -1732,11 +1751,6 @@ int do_command6(int argc, char *argv[], char **table,
 
        generic_opt_check(command, cs.options);
 
-       if (chain != NULL && strlen(chain) >= XT_EXTENSION_MAXNAMELEN)
-               xtables_error(PARAMETER_PROBLEM,
-                          "chain name `%s' too long (must be under %u chars)",
-                          chain, XT_EXTENSION_MAXNAMELEN);
-
        /* Attempt to acquire the xtables lock */
        if (!restore && !xtables_lock(wait)) {
                fprintf(stderr, "Another app is currently holding the xtables lock. "

diff --git a/iptables/iptables.c b/iptables/iptables.c
index d3899bcb2691e8e821e66313943b61334131d5f6..5cd2596e6ed555c6afb382f07476e4eccae4d0b8 100644 (file)
--- a/iptables/iptables.c
+++ b/iptables/iptables.c
@@ -373,6 +373,32 @@ parse_rulenumber(const char *rule)
        return rulenum;
 }
 
+static void
+parse_chain(const char *chainname)
+{
+       const char *ptr;
+
+       if (strlen(chainname) >= XT_EXTENSION_MAXNAMELEN)
+               xtables_error(PARAMETER_PROBLEM,
+                          "chain name `%s' too long (must be under %u chars)",
+                          chainname, XT_EXTENSION_MAXNAMELEN);
+
+       if (*chainname == '-' || *chainname == '!')
+               xtables_error(PARAMETER_PROBLEM,
+                          "chain name not allowed to start "
+                          "with `%c'\n", *chainname);
+
+       if (xtables_find_target(chainname, XTF_TRY_LOAD))
+               xtables_error(PARAMETER_PROBLEM,
+                          "chain name may not clash "
+                          "with target name\n");
+
+       for (ptr = chainname; *ptr; ptr++)
+               if (isspace(*ptr))
+                       xtables_error(PARAMETER_PROBLEM,
+                                  "Invalid chain name `%s'", chainname);
+}
+
 static const char *
 parse_target(const char *targetname)
 {
@@ -1428,14 +1454,7 @@ int do_command4(int argc, char *argv[], char **table,
                        break;
 
                case 'N':
-                       if (optarg && (*optarg == '-' || *optarg == '!'))
-                               xtables_error(PARAMETER_PROBLEM,
-                                          "chain name not allowed to start "
-                                          "with `%c'\n", *optarg);
-                       if (xtables_find_target(optarg, XTF_TRY_LOAD))
-                               xtables_error(PARAMETER_PROBLEM,
-                                          "chain name may not clash "
-                                          "with target name\n");
+                       parse_chain(optarg);
                        add_command(&command, CMD_NEW_CHAIN, CMD_NONE,
                                    cs.invert);
                        chain = optarg;
@@ -1729,11 +1748,6 @@ int do_command4(int argc, char *argv[], char **table,
 
        generic_opt_check(command, cs.options);
 
-       if (chain != NULL && strlen(chain) >= XT_EXTENSION_MAXNAMELEN)
-               xtables_error(PARAMETER_PROBLEM,
-                          "chain name `%s' too long (must be under %u chars)",
-                          chain, XT_EXTENSION_MAXNAMELEN);
-
        /* Attempt to acquire the xtables lock */
        if (!restore && !xtables_lock(wait)) {
                fprintf(stderr, "Another app is currently holding the xtables lock. "