* [iptables PATCH v2] xtables-restore: Fix --table parameter check
@ 2019-10-21 13:23 Phil Sutter
2019-10-21 13:27 ` Florian Westphal
0 siblings, 1 reply; 2+ messages in thread
From: Phil Sutter @ 2019-10-21 13:23 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: netfilter-devel, Florian Westphal
Xtables-restore tries to reject rule commands in input which contain a
--table parameter (since it is adding this itself based on the previous
table line). The manual check was not perfect though as it caught any
parameter starting with a dash and containing a 't' somewhere, even in
rule comments:
| *filter
| -A FORWARD -m comment --comment "- allow this one" -j ACCEPT
| COMMIT
Instead of error-prone manual checking, go a much simpler route: All
do_command callbacks are passed a boolean indicating they're called from
*tables-restore. React upon this when handling a table parameter and
error out if it's not the first one.
Fixes: f8e5ebc5986bf ("iptables: Fix crash on malformed iptables-restore")
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
Changes since v1:
Completely rewritten: While simplifying v1 code, I noticed that even
valid --table options may appear in comments without problem, so
searched for a different approach than manual option parsing. This
solution is much easier and fully backwards compatible.
---
iptables/iptables.c | 4 ++++
| 13 +++++++++++++
iptables/xshared.c | 12 ------------
iptables/xtables-eb.c | 4 ++++
iptables/xtables.c | 4 ++++
5 files changed, 25 insertions(+), 12 deletions(-)
create mode 100755 iptables/tests/shell/testcases/ipt-restore/0009-table-name-comment_0
diff --git a/iptables/iptables.c b/iptables/iptables.c
index 0fbe3ec96bb27..d7a41321760e0 100644
--- a/iptables/iptables.c
+++ b/iptables/iptables.c
@@ -1494,6 +1494,10 @@ int do_command4(int argc, char *argv[], char **table,
if (cs.invert)
xtables_error(PARAMETER_PROBLEM,
"unexpected ! flag before --table");
+ if (restore && *table)
+ xtables_error(PARAMETER_PROBLEM,
+ "The -t option (seen in line %u) cannot be used in %s.\n",
+ line, xt_params->program_name);
*table = optarg;
break;
--git a/iptables/tests/shell/testcases/ipt-restore/0009-table-name-comment_0 b/iptables/tests/shell/testcases/ipt-restore/0009-table-name-comment_0
new file mode 100755
index 0000000000000..4e2202df986cf
--- /dev/null
+++ b/iptables/tests/shell/testcases/ipt-restore/0009-table-name-comment_0
@@ -0,0 +1,13 @@
+#!/bin/bash
+
+# when restoring a ruleset, *tables-restore prefixes each rule with
+# '-t <tablename>' so standard rule parsing routines may be used. This means
+# that it has to detect and reject rules which already contain a table option.
+
+$XT_MULTI iptables-restore <<EOF
+*filter
+-t nat -A FORWARD -j ACCEPT
+COMMIT
+EOF
+
+[[ $? != 0 ]] || exit 1
diff --git a/iptables/xshared.c b/iptables/xshared.c
index ba723f59dbaad..5211b6472ed81 100644
--- a/iptables/xshared.c
+++ b/iptables/xshared.c
@@ -533,18 +533,6 @@ void add_param_to_argv(char *parsestart, int line)
}
param.buffer[param.len] = '\0';
-
- /* check if table name specified */
- if ((param.buffer[0] == '-' &&
- param.buffer[1] != '-' &&
- strchr(param.buffer, 't')) ||
- (!strncmp(param.buffer, "--t", 3) &&
- !strncmp(param.buffer, "--table", strlen(param.buffer)))) {
- xtables_error(PARAMETER_PROBLEM,
- "The -t option (seen in line %u) cannot be used in %s.\n",
- line, xt_params->program_name);
- }
-
add_argv(param.buffer, 0);
param.len = 0;
}
diff --git a/iptables/xtables-eb.c b/iptables/xtables-eb.c
index 3b03daef28eb3..aa754d79608da 100644
--- a/iptables/xtables-eb.c
+++ b/iptables/xtables-eb.c
@@ -947,6 +947,10 @@ print_zero:
break;
case 't': /* Table */
ebt_check_option2(&flags, OPT_TABLE);
+ if (restore && *table)
+ xtables_error(PARAMETER_PROBLEM,
+ "The -t option (seen in line %u) cannot be used in %s.\n",
+ line, xt_params->program_name);
if (strlen(optarg) > EBT_TABLE_MAXNAMELEN - 1)
xtables_error(PARAMETER_PROBLEM,
"Table name length cannot exceed %d characters",
diff --git a/iptables/xtables.c b/iptables/xtables.c
index 0e0cb5f53d421..89f3271e36dd0 100644
--- a/iptables/xtables.c
+++ b/iptables/xtables.c
@@ -879,6 +879,10 @@ void do_parse(struct nft_handle *h, int argc, char *argv[],
if (cs->invert)
xtables_error(PARAMETER_PROBLEM,
"unexpected ! flag before --table");
+ if (p->restore && p->table)
+ xtables_error(PARAMETER_PROBLEM,
+ "The -t option (seen in line %u) cannot be used in %s.\n",
+ line, xt_params->program_name);
if (!nft_table_builtin_find(h, optarg))
xtables_error(VERSION_PROBLEM,
"table '%s' does not exist",
--
2.23.0
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [iptables PATCH v2] xtables-restore: Fix --table parameter check
2019-10-21 13:23 [iptables PATCH v2] xtables-restore: Fix --table parameter check Phil Sutter
@ 2019-10-21 13:27 ` Florian Westphal
0 siblings, 0 replies; 2+ messages in thread
From: Florian Westphal @ 2019-10-21 13:27 UTC (permalink / raw)
To: Phil Sutter; +Cc: Pablo Neira Ayuso, netfilter-devel, Florian Westphal
Phil Sutter <phil@nwl.cc> wrote:
> Xtables-restore tries to reject rule commands in input which contain a
> --table parameter (since it is adding this itself based on the previous
> table line). The manual check was not perfect though as it caught any
> parameter starting with a dash and containing a 't' somewhere, even in
> rule comments:
>
> | *filter
> | -A FORWARD -m comment --comment "- allow this one" -j ACCEPT
> | COMMIT
>
> Instead of error-prone manual checking, go a much simpler route: All
> do_command callbacks are passed a boolean indicating they're called from
> *tables-restore. React upon this when handling a table parameter and
> error out if it's not the first one.
>
> if (cs.invert)
> xtables_error(PARAMETER_PROBLEM,
> "unexpected ! flag before --table");
> + if (restore && *table)
> + xtables_error(PARAMETER_PROBLEM,
> + "The -t option (seen in line %u) cannot be used in %s.\n",
> + line, xt_params->program_name);
Oh, thats much better indeed.
Acked-by: Florian Westphal <fw@strlen.de>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2019-10-21 13:27 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-21 13:23 [iptables PATCH v2] xtables-restore: Fix --table parameter check Phil Sutter
2019-10-21 13:27 ` Florian Westphal
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).