From 90a7a183a208b691810b8519cc57d3d9d3b7eb60 Mon Sep 17 00:00:00 2001 From: Phil Sutter Date: Fri, 28 Apr 2023 14:41:08 +0200 Subject: xshared: Fix parsing of option arguments in same word When merging commandline parsers, a decision between 'argv[optind - 1]' and 'optarg' had to be made in some spots. While the implementation of check_inverse() required the former, use of the latter allows for the common syntax of '--opt=arg' or even '-oarg' as 'optarg' will point at the suffix while 'argv[optind - 1]' will just point at the following option. Fix the mess by making check_inverse() update optarg pointer if needed so calling code may refer to and always correct 'optarg'. Fixes: 0af80a91b0a98 ("nft: Merge xtables-arp-standalone.c into xtables-standalone.c") Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1677 Signed-off-by: Phil Sutter --- extensions/libarpt_standard.t | 2 ++ extensions/libxt_standard.t | 3 +++ iptables/xshared.c | 61 +++++++++++++++++++++---------------------- 3 files changed, 35 insertions(+), 31 deletions(-) diff --git a/extensions/libarpt_standard.t b/extensions/libarpt_standard.t index e84a00b7..007fa2b8 100644 --- a/extensions/libarpt_standard.t +++ b/extensions/libarpt_standard.t @@ -12,3 +12,5 @@ -i lo --destination-mac 11:22:33:44:55:66;-i lo --dst-mac 11:22:33:44:55:66;OK --source-mac Unicast;--src-mac 00:00:00:00:00:00/01:00:00:00:00:00;OK ! --src-mac Multicast;! --src-mac 01:00:00:00:00:00/01:00:00:00:00:00;OK +--src-mac=01:02:03:04:05:06 --dst-mac=07:08:09:0A:0B:0C --h-length=6 --opcode=Request --h-type=Ethernet --proto-type=ipv4;--src-mac 01:02:03:04:05:06 --dst-mac 07:08:09:0a:0b:0c --opcode 1 --proto-type 0x800;OK +--src-mac ! 01:02:03:04:05:06 --dst-mac ! 07:08:09:0A:0B:0C --h-length ! 6 --opcode ! Request --h-type ! Ethernet --proto-type ! ipv4;! --src-mac 01:02:03:04:05:06 ! --dst-mac 07:08:09:0a:0b:0c ! --h-length 6 ! --opcode 1 ! --h-type 1 ! --proto-type 0x800;OK diff --git a/extensions/libxt_standard.t b/extensions/libxt_standard.t index 56d6da2e..6ed978e4 100644 --- a/extensions/libxt_standard.t +++ b/extensions/libxt_standard.t @@ -21,3 +21,6 @@ -s 10.11.12.13/255.128.0.0;-s 10.0.0.0/9;OK -s 10.11.12.13/255.0.255.0;-s 10.0.12.0/255.0.255.0;OK -s 10.11.12.13/255.0.12.0;-s 10.0.12.0/255.0.12.0;OK +:FORWARD +--protocol=tcp --source=1.2.3.4 --destination=5.6.7.8/32 --in-interface=eth0 --out-interface=eth1 --jump=ACCEPT;-s 1.2.3.4/32 -d 5.6.7.8/32 -i eth0 -o eth1 -p tcp -j ACCEPT;OK +-ptcp -s1.2.3.4 -d5.6.7.8/32 -ieth0 -oeth1 -jACCEPT;-s 1.2.3.4/32 -d 5.6.7.8/32 -i eth0 -o eth1 -p tcp -j ACCEPT;OK diff --git a/iptables/xshared.c b/iptables/xshared.c index ac51fac5..17aed04e 100644 --- a/iptables/xshared.c +++ b/iptables/xshared.c @@ -1318,7 +1318,7 @@ static void check_empty_interface(struct xtables_args *args, const char *arg) } static void check_inverse(struct xtables_args *args, const char option[], - bool *invert, int *optidx, int argc) + bool *invert, int argc, char **argv) { switch (args->family) { case NFPROTO_ARP: @@ -1337,12 +1337,11 @@ static void check_inverse(struct xtables_args *args, const char option[], xtables_error(PARAMETER_PROBLEM, "Multiple `!' flags not allowed"); *invert = true; - if (optidx) { - *optidx = *optidx + 1; - if (argc && *optidx > argc) - xtables_error(PARAMETER_PROBLEM, - "no argument following `!'"); - } + optind++; + if (optind > argc) + xtables_error(PARAMETER_PROBLEM, "no argument following `!'"); + + optarg = argv[optind - 1]; } static const char *optstring_lookup(int family) @@ -1555,16 +1554,16 @@ void do_parse(int argc, char *argv[], * Option selection */ case 'p': - check_inverse(args, optarg, &invert, &optind, argc); + check_inverse(args, optarg, &invert, argc, argv); set_option(&cs->options, OPT_PROTOCOL, &args->invflags, invert); /* Canonicalize into lower case */ - for (cs->protocol = argv[optind - 1]; + for (cs->protocol = optarg; *cs->protocol; cs->protocol++) *cs->protocol = tolower(*cs->protocol); - cs->protocol = argv[optind - 1]; + cs->protocol = optarg; args->proto = xtables_parse_protocol(cs->protocol); if (args->proto == 0 && @@ -1578,17 +1577,17 @@ void do_parse(int argc, char *argv[], break; case 's': - check_inverse(args, optarg, &invert, &optind, argc); + check_inverse(args, optarg, &invert, argc, argv); set_option(&cs->options, OPT_SOURCE, &args->invflags, invert); - args->shostnetworkmask = argv[optind - 1]; + args->shostnetworkmask = optarg; break; case 'd': - check_inverse(args, optarg, &invert, &optind, argc); + check_inverse(args, optarg, &invert, argc, argv); set_option(&cs->options, OPT_DESTINATION, &args->invflags, invert); - args->dhostnetworkmask = argv[optind - 1]; + args->dhostnetworkmask = optarg; break; #ifdef IPT_F_GOTO @@ -1601,71 +1600,71 @@ void do_parse(int argc, char *argv[], #endif case 2:/* src-mac */ - check_inverse(args, optarg, &invert, &optind, argc); + check_inverse(args, optarg, &invert, argc, argv); set_option(&cs->options, OPT_S_MAC, &args->invflags, invert); - args->src_mac = argv[optind - 1]; + args->src_mac = optarg; break; case 3:/* dst-mac */ - check_inverse(args, optarg, &invert, &optind, argc); + check_inverse(args, optarg, &invert, argc, argv); set_option(&cs->options, OPT_D_MAC, &args->invflags, invert); - args->dst_mac = argv[optind - 1]; + args->dst_mac = optarg; break; case 'l':/* hardware length */ - check_inverse(args, optarg, &invert, &optind, argc); + check_inverse(args, optarg, &invert, argc, argv); set_option(&cs->options, OPT_H_LENGTH, &args->invflags, invert); - args->arp_hlen = argv[optind - 1]; + args->arp_hlen = optarg; break; case 8: /* was never supported, not even in arptables-legacy */ xtables_error(PARAMETER_PROBLEM, "not supported"); case 4:/* opcode */ - check_inverse(args, optarg, &invert, &optind, argc); + check_inverse(args, optarg, &invert, argc, argv); set_option(&cs->options, OPT_OPCODE, &args->invflags, invert); - args->arp_opcode = argv[optind - 1]; + args->arp_opcode = optarg; break; case 5:/* h-type */ - check_inverse(args, optarg, &invert, &optind, argc); + check_inverse(args, optarg, &invert, argc, argv); set_option(&cs->options, OPT_H_TYPE, &args->invflags, invert); - args->arp_htype = argv[optind - 1]; + args->arp_htype = optarg; break; case 6:/* proto-type */ - check_inverse(args, optarg, &invert, &optind, argc); + check_inverse(args, optarg, &invert, argc, argv); set_option(&cs->options, OPT_P_TYPE, &args->invflags, invert); - args->arp_ptype = argv[optind - 1]; + args->arp_ptype = optarg; break; case 'j': set_option(&cs->options, OPT_JUMP, &args->invflags, invert); - command_jump(cs, argv[optind - 1]); + command_jump(cs, optarg); break; case 'i': check_empty_interface(args, optarg); - check_inverse(args, optarg, &invert, &optind, argc); + check_inverse(args, optarg, &invert, argc, argv); set_option(&cs->options, OPT_VIANAMEIN, &args->invflags, invert); - xtables_parse_interface(argv[optind - 1], + xtables_parse_interface(optarg, args->iniface, args->iniface_mask); break; case 'o': check_empty_interface(args, optarg); - check_inverse(args, optarg, &invert, &optind, argc); + check_inverse(args, optarg, &invert, argc, argv); set_option(&cs->options, OPT_VIANAMEOUT, &args->invflags, invert); - xtables_parse_interface(argv[optind - 1], + xtables_parse_interface(optarg, args->outiface, args->outiface_mask); break; -- cgit v1.2.3