Re: [PATCH] extensions: ebt_string: Remove spurious inverse check for --string-icase

Re: [PATCH] extensions: ebt_string: Remove spurious inverse check for --string-icase

[Florian Westphal]

YuPeng <yupeng@kylinos.cn> wrote:
> --string-icase is registered as a no_argument option, so getopt_long
> always sets optarg to NULL when it is parsed.  The ebt_check_inverse2()
> macro contains a mandatory-argument guard:
> 
> 	if (!optarg) {
> 		__ebt_print_error("Option without (mandatory) argument");
> 		return -1;
> 	}
> 
> so every use of --string-icase aborts parsing with the misleading error
> "Option without (mandatory) argument." and XT_STRING_FLAG_IGNORECASE
> can never be set.  The option has been broken since it was added in
> commit b1cdae8 ("extensions: Add string filter to ebtables").
> 
> Reproducer (no privileges required):
> 
>   $ ebtables-legacy --string-icase
>   Option without (mandatory) argument.
> 
> The inverse check makes no sense for a flag option that takes no
> argument, so simply remove it.
> 
> This fix was originally carried in the Kylin Linux SRPM since 2022;
> the commit message has been rewritten to describe the actual failure
> mode observed on current master.

> diff --git a/extensions/ebt_string.c b/extensions/ebt_string.c
> index 97fbe19..74a1bfa 100644
> --- a/extensions/ebt_string.c
> +++ b/extensions/ebt_string.c
> @@ -174,8 +174,6 @@ static int parse(int c, char **argv, int argc, const struct ebt_u_entry *entry,
>  		break;
>  	case STRING_ICASE:
>  		ebt_check_option2(flags, OPT_STRING_ICASE);
> -		if (ebt_check_inverse2(optarg))
> -			ebt_print_error2("Unexpected `!' after --string-icase");

Are you sure?  Probably:
-		if (ebt_check_inverse2(optarg))
+		if (ebt_check_inverse(optarg))

?

[PATCH] extensions: ebt_string: Remove spurious inverse check for --string-icase

[YuPeng]

--string-icase is registered as a no_argument option, so getopt_long
always sets optarg to NULL when it is parsed.  The ebt_check_inverse2()
macro contains a mandatory-argument guard:

	if (!optarg) {
		__ebt_print_error("Option without (mandatory) argument");
		return -1;
	}

so every use of --string-icase aborts parsing with the misleading error
"Option without (mandatory) argument." and XT_STRING_FLAG_IGNORECASE
can never be set.  The option has been broken since it was added in
commit b1cdae8 ("extensions: Add string filter to ebtables").

Reproducer (no privileges required):

  $ ebtables-legacy --string-icase
  Option without (mandatory) argument.

The inverse check makes no sense for a flag option that takes no
argument, so simply remove it.

This fix was originally carried in the Kylin Linux SRPM since 2022;
the commit message has been rewritten to describe the actual failure
mode observed on current master.

Signed-off-by: YuPeng <yupeng@kylinos.cn>
---
 extensions/ebt_string.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/extensions/ebt_string.c b/extensions/ebt_string.c
index 97fbe19..74a1bfa 100644
--- a/extensions/ebt_string.c
+++ b/extensions/ebt_string.c
@@ -174,8 +174,6 @@ static int parse(int c, char **argv, int argc, const struct ebt_u_entry *entry,
 		break;
 	case STRING_ICASE:
 		ebt_check_option2(flags, OPT_STRING_ICASE);
-		if (ebt_check_inverse2(optarg))
-			ebt_print_error2("Unexpected `!' after --string-icase");
 		info->u.v1.flags |= XT_STRING_FLAG_IGNORECASE;
 		break;
 	case STRING:
-- 
2.39.5