Hi!
On 5/15/19 10:31 PM, Phil Sutter wrote:
> Hi,
>
> On Wed, May 15, 2019 at 09:56:11PM +0200, Fernando Fernandez Mancera wrote:
>> Hi Phil,
>>
>> On 5/15/19 9:26 PM, Phil Sutter wrote:
>>> Hi Pablo,
>>>
>>> On Wed, May 15, 2019 at 05:21:32PM +0200, Pablo Neira Ayuso wrote:
>>>> On Wed, May 15, 2019 at 01:46:17PM +0200, Phil Sutter wrote>> [...]
>>>> '@<something>' is currently allowed, as any arbitrary string can be
>>>> placed in between strings - although in some way this is taking us
>>>> back to the quote debate that needs to be addressed. If we want to
>>>> disallow something enclosed in quotes then we'll have to apply this
>>>> function everywhere we allow variables.
>>>
>>> Oh, sorry. I put those ticks in there just to quote the value, not as
>>> part of the value. The intention was to point out that something like:
>>>
>>> | define foo = @set1
>>> | add rule ip t c jump $foo
>>>
>>> Might pass evaluation stage and since there is a special case for things
>>> starting with '@' in symbol_expr, the added rule would turn into
>>>
>>> | add rule ip t c jump set1
>>>
>>> We could detect this situation by checking expr->symtype.
>>>
>>
>> I agree about that. We could check if the symbol type is SYMBOL_VALUE.
>> But I am not sure about where should we do it, maybe in the parser?
>>
>>> On the other hand, can we maybe check if given string points to an
>>> *existing* chain in verdict_type_parse()? Or will that happen later
>>> anyway?
>>>
>>
>> It happens later, right now if the given string does not point to an
>> existing chain it returns the usual error for this situation. e.g
>
> I just played around a bit and could provoke some segfaults:
>
> * define foo = @set1 (a set named 'set1' must exist)
> * define foo = { 1024 }
> * define foo = *
>
> I didn't check how we could avoid those. Maybe this is even follow-up
> work, but we should definitely try to address those eventually.
>
I have been working on fixing this. I propose the following fix.
diff --git a/src/evaluate.c b/src/evaluate.c
index 8394037..edab370 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -1950,6 +1950,12 @@ static int stmt_evaluate_verdict(struct eval_ctx
*ctx, struct stmt *stmt)
if (stmt->expr->chain != NULL) {
if (expr_evaluate(ctx, &stmt->expr->chain) < 0)
return -1;
+ if (stmt->expr->chain->etype != EXPR_SYMBOL ||
+ stmt->expr->chain->symtype != SYMBOL_VALUE) {
+ BUG("invalid verdict chain expression %s\n",
+ expr_name(stmt->expr->chain));
+ return -1;
+ }
}
break;
case EXPR_MAP:
That works for all the cases that you mentioned above. What do you think?
Thanks :-)
> Cheers, Phil
>
