diff options
author | Phil Oester <kernel@linuxace.com> | 2013-10-05 09:33:15 -0700 |
---|---|---|
committer | Pablo Neira Ayuso <pablo@netfilter.org> | 2013-11-03 21:05:19 +0100 |
commit | e0853f3c285a6ad082e5ed5ff5e308beeeba41d9 (patch) | |
tree | 134a711d4bd7dc6aa65c79cc50e8856c357d1bfa | |
parent | b90e798596465f0ea2daff75d95f4f978f0a8377 (diff) |
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>
-rw-r--r-- | iptables/ip6tables.c | 40 | ||||
-rw-r--r-- | iptables/iptables.c | 40 |
2 files changed, 54 insertions, 26 deletions
diff --git a/iptables/ip6tables.c b/iptables/ip6tables.c index 76de3672..a5199d5e 100644 --- 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 d3899bcb..5cd2596e 100644 --- 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. " |