* [nft PATCH] evaluate: Reject quoted strings containing only wildcard
@ 2020-09-24 17:06 Phil Sutter
2020-09-28 8:19 ` Phil Sutter
0 siblings, 1 reply; 5+ messages in thread
From: Phil Sutter @ 2020-09-24 17:06 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: netfilter-devel
Fix for an assertion fail when trying to match against an all-wildcard
interface name:
| % nft add rule t c iifname '"*"'
| nft: expression.c:402: constant_expr_alloc: Assertion `(((len) + (8) - 1) / (8)) > 0' failed.
| zsh: abort nft add rule t c iifname '"*"'
Fix this by detecting the string in expr_evaluate_string() and returning
an error message:
| % nft add rule t c iifname '"*"'
| Error: All-wildcard strings are not supported
| add rule t c iifname "*"
| ^^^
While being at it, drop the 'datalen >= 1' clause from the following
conditional as together with the added check for 'datalen == 0', all
possible other values have been caught already.
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
src/evaluate.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/src/evaluate.c b/src/evaluate.c
index c8045e5ded729..5f17d7501ac0e 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -324,8 +324,11 @@ static int expr_evaluate_string(struct eval_ctx *ctx, struct expr **exprp)
return 0;
}
- if (datalen >= 1 &&
- data[datalen - 1] == '\\') {
+ if (datalen == 0)
+ return expr_error(ctx->msgs, expr,
+ "All-wildcard strings are not supported");
+
+ if (data[datalen - 1] == '\\') {
char unescaped_str[data_len];
memset(unescaped_str, 0, sizeof(unescaped_str));
--
2.28.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [nft PATCH] evaluate: Reject quoted strings containing only wildcard
2020-09-24 17:06 [nft PATCH] evaluate: Reject quoted strings containing only wildcard Phil Sutter
@ 2020-09-28 8:19 ` Phil Sutter
2020-09-30 10:20 ` Pablo Neira Ayuso
0 siblings, 1 reply; 5+ messages in thread
From: Phil Sutter @ 2020-09-28 8:19 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: netfilter-devel
On Thu, Sep 24, 2020 at 07:06:39PM +0200, Phil Sutter wrote:
> Fix for an assertion fail when trying to match against an all-wildcard
> interface name:
>
> | % nft add rule t c iifname '"*"'
> | nft: expression.c:402: constant_expr_alloc: Assertion `(((len) + (8) - 1) / (8)) > 0' failed.
> | zsh: abort nft add rule t c iifname '"*"'
>
> Fix this by detecting the string in expr_evaluate_string() and returning
> an error message:
>
> | % nft add rule t c iifname '"*"'
> | Error: All-wildcard strings are not supported
> | add rule t c iifname "*"
> | ^^^
>
Note that all this is pretty inconsistent: The above happens only for
quoted asterisks. Unquoted ones cause a different error (at least no
assertion fail):
| % nft add rule t c iifname '*'
| Error: datatype mismatch, expected network interface name, expression has type integer
| add rule t c iifname *
| ~~~~~~~ ^
What puzzles me is that we have:
| wildcard_expr : ASTERISK
| {
| struct expr *expr;
|
| expr = constant_expr_alloc(&@$, &integer_type,
| BYTEORDER_HOST_ENDIAN,
| 0, NULL);
| $$ = prefix_expr_alloc(&@$, expr, 0);
| }
| ;
Yet when trying to use it as a prefix, it is rejected:
| % nft add rule t c ip saddr '*'
| Error: datatype mismatch, expected IPv4 address, expression has type integer
| add rule t c ip saddr *
| ~~~~~~~~ ^
So is this wildcard_expr simply broken or didn't I find correct way to use it
yet?
Cheers, Phil
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [nft PATCH] evaluate: Reject quoted strings containing only wildcard
2020-09-28 8:19 ` Phil Sutter
@ 2020-09-30 10:20 ` Pablo Neira Ayuso
2020-12-17 14:23 ` Phil Sutter
0 siblings, 1 reply; 5+ messages in thread
From: Pablo Neira Ayuso @ 2020-09-30 10:20 UTC (permalink / raw)
To: Phil Sutter, netfilter-devel
On Mon, Sep 28, 2020 at 10:19:25AM +0200, Phil Sutter wrote:
> On Thu, Sep 24, 2020 at 07:06:39PM +0200, Phil Sutter wrote:
> > Fix for an assertion fail when trying to match against an all-wildcard
> > interface name:
> >
> > | % nft add rule t c iifname '"*"'
> > | nft: expression.c:402: constant_expr_alloc: Assertion `(((len) + (8) - 1) / (8)) > 0' failed.
> > | zsh: abort nft add rule t c iifname '"*"'
> >
> > Fix this by detecting the string in expr_evaluate_string() and returning
> > an error message:
> >
> > | % nft add rule t c iifname '"*"'
> > | Error: All-wildcard strings are not supported
> > | add rule t c iifname "*"
> > | ^^^
> >
>
> Note that all this is pretty inconsistent: The above happens only for
> quoted asterisks. Unquoted ones cause a different error (at least no
> assertion fail):
>
> | % nft add rule t c iifname '*'
> | Error: datatype mismatch, expected network interface name, expression has type integer
> | add rule t c iifname *
> | ~~~~~~~ ^
>
> What puzzles me is that we have:
>
> | wildcard_expr : ASTERISK
> | {
> | struct expr *expr;
> |
> | expr = constant_expr_alloc(&@$, &integer_type,
> | BYTEORDER_HOST_ENDIAN,
> | 0, NULL);
> | $$ = prefix_expr_alloc(&@$, expr, 0);
> | }
> | ;
>
> Yet when trying to use it as a prefix, it is rejected:
>
> | % nft add rule t c ip saddr '*'
> | Error: datatype mismatch, expected IPv4 address, expression has type integer
> | add rule t c ip saddr *
> | ~~~~~~~~ ^
>
> So is this wildcard_expr simply broken or didn't I find correct way to use it
> yet?
This looks like some preliminary support for wildcard matching in set
elements, but my impression is that this is broken. I don't remember
to have seen any tests covering this.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [nft PATCH] evaluate: Reject quoted strings containing only wildcard
2020-09-30 10:20 ` Pablo Neira Ayuso
@ 2020-12-17 14:23 ` Phil Sutter
2020-12-17 14:32 ` Pablo Neira Ayuso
0 siblings, 1 reply; 5+ messages in thread
From: Phil Sutter @ 2020-12-17 14:23 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: netfilter-devel
On Wed, Sep 30, 2020 at 12:20:33PM +0200, Pablo Neira Ayuso wrote:
> On Mon, Sep 28, 2020 at 10:19:25AM +0200, Phil Sutter wrote:
> > On Thu, Sep 24, 2020 at 07:06:39PM +0200, Phil Sutter wrote:
> > > Fix for an assertion fail when trying to match against an all-wildcard
> > > interface name:
> > >
> > > | % nft add rule t c iifname '"*"'
> > > | nft: expression.c:402: constant_expr_alloc: Assertion `(((len) + (8) - 1) / (8)) > 0' failed.
> > > | zsh: abort nft add rule t c iifname '"*"'
> > >
> > > Fix this by detecting the string in expr_evaluate_string() and returning
> > > an error message:
> > >
> > > | % nft add rule t c iifname '"*"'
> > > | Error: All-wildcard strings are not supported
> > > | add rule t c iifname "*"
> > > | ^^^
> > >
> >
> > Note that all this is pretty inconsistent: The above happens only for
> > quoted asterisks. Unquoted ones cause a different error (at least no
> > assertion fail):
> >
> > | % nft add rule t c iifname '*'
> > | Error: datatype mismatch, expected network interface name, expression has type integer
> > | add rule t c iifname *
> > | ~~~~~~~ ^
> >
> > What puzzles me is that we have:
> >
> > | wildcard_expr : ASTERISK
> > | {
> > | struct expr *expr;
> > |
> > | expr = constant_expr_alloc(&@$, &integer_type,
> > | BYTEORDER_HOST_ENDIAN,
> > | 0, NULL);
> > | $$ = prefix_expr_alloc(&@$, expr, 0);
> > | }
> > | ;
> >
> > Yet when trying to use it as a prefix, it is rejected:
> >
> > | % nft add rule t c ip saddr '*'
> > | Error: datatype mismatch, expected IPv4 address, expression has type integer
> > | add rule t c ip saddr *
> > | ~~~~~~~~ ^
> >
> > So is this wildcard_expr simply broken or didn't I find correct way to use it
> > yet?
>
> This looks like some preliminary support for wildcard matching in set
> elements, but my impression is that this is broken. I don't remember
> to have seen any tests covering this.
OK. If it needs fixing, I guess that's a different issue. Are you fine
with the "fix" for asterisk-only interface names for the time being?
Thanks, Phil
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [nft PATCH] evaluate: Reject quoted strings containing only wildcard
2020-12-17 14:23 ` Phil Sutter
@ 2020-12-17 14:32 ` Pablo Neira Ayuso
0 siblings, 0 replies; 5+ messages in thread
From: Pablo Neira Ayuso @ 2020-12-17 14:32 UTC (permalink / raw)
To: Phil Sutter, netfilter-devel
On Thu, Dec 17, 2020 at 03:23:40PM +0100, Phil Sutter wrote:
> On Wed, Sep 30, 2020 at 12:20:33PM +0200, Pablo Neira Ayuso wrote:
> > On Mon, Sep 28, 2020 at 10:19:25AM +0200, Phil Sutter wrote:
> > > On Thu, Sep 24, 2020 at 07:06:39PM +0200, Phil Sutter wrote:
> > > > Fix for an assertion fail when trying to match against an all-wildcard
> > > > interface name:
> > > >
> > > > | % nft add rule t c iifname '"*"'
> > > > | nft: expression.c:402: constant_expr_alloc: Assertion `(((len) + (8) - 1) / (8)) > 0' failed.
> > > > | zsh: abort nft add rule t c iifname '"*"'
> > > >
> > > > Fix this by detecting the string in expr_evaluate_string() and returning
> > > > an error message:
> > > >
> > > > | % nft add rule t c iifname '"*"'
> > > > | Error: All-wildcard strings are not supported
> > > > | add rule t c iifname "*"
> > > > | ^^^
> > > >
> > >
> > > Note that all this is pretty inconsistent: The above happens only for
> > > quoted asterisks. Unquoted ones cause a different error (at least no
> > > assertion fail):
> > >
> > > | % nft add rule t c iifname '*'
> > > | Error: datatype mismatch, expected network interface name, expression has type integer
> > > | add rule t c iifname *
> > > | ~~~~~~~ ^
> > >
> > > What puzzles me is that we have:
> > >
> > > | wildcard_expr : ASTERISK
> > > | {
> > > | struct expr *expr;
> > > |
> > > | expr = constant_expr_alloc(&@$, &integer_type,
> > > | BYTEORDER_HOST_ENDIAN,
> > > | 0, NULL);
> > > | $$ = prefix_expr_alloc(&@$, expr, 0);
> > > | }
> > > | ;
> > >
> > > Yet when trying to use it as a prefix, it is rejected:
> > >
> > > | % nft add rule t c ip saddr '*'
> > > | Error: datatype mismatch, expected IPv4 address, expression has type integer
> > > | add rule t c ip saddr *
> > > | ~~~~~~~~ ^
> > >
> > > So is this wildcard_expr simply broken or didn't I find correct way to use it
> > > yet?
> >
> > This looks like some preliminary support for wildcard matching in set
> > elements, but my impression is that this is broken. I don't remember
> > to have seen any tests covering this.
>
> OK. If it needs fixing, I guess that's a different issue. Are you fine
> with the "fix" for asterisk-only interface names for the time being?
I think so, yes.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-12-17 14:33 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-24 17:06 [nft PATCH] evaluate: Reject quoted strings containing only wildcard Phil Sutter
2020-09-28 8:19 ` Phil Sutter
2020-09-30 10:20 ` Pablo Neira Ayuso
2020-12-17 14:23 ` Phil Sutter
2020-12-17 14:32 ` Pablo Neira Ayuso
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).