From 148131f20421046fea028e638581e938ec985783 Mon Sep 17 00:00:00 2001 From: Phil Sutter Date: Mon, 4 Feb 2019 21:52:53 +0100 Subject: xtables: Fix for false-positive rule matching When comparing two rules with non-standard targets, differences in targets' payloads wasn't respected. The cause is a rather hideous one: Unlike xtables_find_match(), xtables_find_target() did not care whether the found target was already in use or not, so the same target instance was assigned to both rules and therefore payload comparison happened over the same memory location. With legacy iptables it is not possible to reuse a target: The only case where two rules (i.e., iptables_command_state instances) could exist at the same time is when comparing rules, but that's handled using libiptc. The above change clashes with ebtables-nft's reuse of target objects: While input parsing still just assigns the object from xtables_targets list, rule conversion from nftnl to iptables_command_state allocates new data. To fix this, make ebtables-nft input parsing use the common command_jump() routine instead of its own simplified copy. In turn, this also eliminates the ebtables-nft-specific variants of parse_target(), though with a slight change of behaviour: Names of user-defined chains are no longer allowed to contain up to 31 but merely 28 characters. Signed-off-by: Phil Sutter Signed-off-by: Florian Westphal --- iptables/xtables-eb.c | 47 +---------------------------------------------- 1 file changed, 1 insertion(+), 46 deletions(-) (limited to 'iptables/xtables-eb.c') diff --git a/iptables/xtables-eb.c b/iptables/xtables-eb.c index b9d98a05..75d43963 100644 --- a/iptables/xtables-eb.c +++ b/iptables/xtables-eb.c @@ -139,27 +139,6 @@ static int parse_rule_number(const char *rule) return rule_nr; } -static const char * -parse_target(const char *targetname) -{ - const char *ptr; - - if (strlen(targetname) < 1) - xtables_error(PARAMETER_PROBLEM, - "Invalid target name (too short)"); - - if (strlen(targetname)+1 > EBT_CHAIN_MAXNAMELEN) - xtables_error(PARAMETER_PROBLEM, - "Invalid target '%s' (%d chars max)", - targetname, EBT_CHAIN_MAXNAMELEN); - - for (ptr = targetname; *ptr; ptr++) - if (isspace(*ptr)) - xtables_error(PARAMETER_PROBLEM, - "Invalid target name `%s'", targetname); - return targetname; -} - static int append_entry(struct nft_handle *h, const char *chain, @@ -365,29 +344,6 @@ static struct option *merge_options(struct option *oldopts, return merge; } -/* - * More glue code. - */ -struct xtables_target *ebt_command_jump(const char *jumpto) -{ - struct xtables_target *target; - unsigned int verdict; - - /* Standard target? */ - if (!ebt_fill_target(jumpto, &verdict)) - jumpto = "standard"; - - /* For ebtables, all targets are preloaded. Hence it is either in - * xtables_targets or a custom chain to jump to, in which case - * returning NULL is fine. */ - for (target = xtables_targets; target; target = target->next) { - if (!strcmp(target->name, jumpto)) - break; - } - - return target; -} - static void print_help(const struct xtables_target *t, const struct xtables_rule_match *m, const char *table) { @@ -1055,8 +1011,7 @@ print_zero: } else if (c == 'j') { ebt_check_option2(&flags, OPT_JUMP); if (strcmp(optarg, "CONTINUE") != 0) { - cs.jumpto = parse_target(optarg); - cs.target = ebt_command_jump(cs.jumpto); + command_jump(&cs); } break; } else if (c == 's') { -- cgit v1.2.3