From 6460f4bf097e6c8fcac36962f86d34b07a6aa7ea Mon Sep 17 00:00:00 2001 From: rabbitstack Date: Tue, 5 May 2026 18:53:42 +0200 Subject: [PATCH] fix(filter): Correct infix/unary negation Unary negation was negating the entire binary expression leading to unexpected results. As part of this change, the parser is refactored to handle infix/unary grammar in a more elegant way. --- pkg/filter/filter_test.go | 4 +++ pkg/filter/ql/parser.go | 69 +++++++++++++++++------------------- pkg/filter/ql/parser_test.go | 4 +++ 3 files changed, 41 insertions(+), 36 deletions(-) diff --git a/pkg/filter/filter_test.go b/pkg/filter/filter_test.go index d42c1e5c3..7c7fdaace 100644 --- a/pkg/filter/filter_test.go +++ b/pkg/filter/filter_test.go @@ -648,6 +648,10 @@ func TestFileFilter(t *testing.T) { {`file.extension not contains '.exe' and file.extension not contains '.com' and file.extension not in ('.vba', '.exe')`, true}, {`file.extension not in ('.exe', '.com')`, true}, {`file.extension not in ('.exe', '.dll')`, false}, + {`file.extension = '.dll' and not (file.extension contains '.com') and file.operation = 'open'`, true}, + {`file.extension = '.dll' and not (file.extension contains '.dll' and file.type = 'file')`, false}, + {`file.extension = '.dll' and not (file.extension contains '.exe' and file.type = 'file') and not (file.type = 'directory')`, true}, + {`file.path not imatches 'C:\\Windows\\system32\\*.exe' and file.path not imatches '?:\\Windows\\System32\\svchost.exe'`, true}, {`file.path matches 'C:\\*\\user32.dll'`, true}, {`file.path not matches 'C:\\*.exe'`, true}, {`file.path imatches 'C:\\*\\USER32.dll'`, true}, diff --git a/pkg/filter/ql/parser.go b/pkg/filter/ql/parser.go index 99399de4d..d8edda461 100644 --- a/pkg/filter/ql/parser.go +++ b/pkg/filter/ql/parser.go @@ -244,43 +244,31 @@ func (p *Parser) ParseExpr() (Expr, error) { } } - var rhs Expr - switch op { - case Not: - // the first variant of the negation operator. - // The operator that is negated appears immediately - // after the `not` operator, e.g. ps.name not in ('cmd.exe') + if op == Not { + // handle infix negation op1, pos, lit := p.scanIgnoreWhitespace() if !op1.isOperator() { - return nil, newParseError(tokstr(op, lit), []string{"operator"}, pos, p.expr) + return nil, newParseError(tokstr(op1, lit), []string{"operator"}, pos, p.expr) } - // parse the next expression after operator - rhs1, err := p.parseUnaryExpr() + rhs, err := p.parseUnaryExpr() if err != nil { return nil, err } - rhs = &BinaryExpr{RHS: rhs1, Op: op1} - default: - op1, _, _ := p.scanIgnoreWhitespace() - // if the negation appears after the operator - // try to parse an entire binary expr and wrap - // it inside the `not` expression. This is the - // second variant of the negating expressions, e.g. - // ps.name = 'cmd.exe' and not (ps.name in ('powershell.exe')) - if op1 == Not { - binaryExpr, err := p.ParseExpr() - if err != nil { - return nil, err - } - rhs = &NotExpr{binaryExpr} - } else { - p.unscan() - // otherwise, parse the next expression - rhs, err = p.parseUnaryExpr() - if err != nil { - return nil, err + + for node := root; ; { + r, ok := node.RHS.(*BinaryExpr) + if !ok || r.Op.precedence() >= op1.precedence() { + node.RHS = &NotExpr{Expr: &BinaryExpr{LHS: node.RHS, RHS: rhs, Op: op1}} + break } + node = r } + continue + } + + rhs, err := p.parseUnaryExpr() + if err != nil { + return nil, err } // find the right spot in the tree to add the new expression by @@ -290,13 +278,7 @@ func (p *Parser) ParseExpr() (Expr, error) { for node := root; ; { r, ok := node.RHS.(*BinaryExpr) if !ok || r.Op.precedence() >= op.precedence() { - if op == Not { - r := rhs.(*BinaryExpr) - r.LHS = node.RHS - node.RHS = &NotExpr{Expr: r} - break - } - // Add the new expression here and break. + // add the new expression here and break node.RHS = &BinaryExpr{LHS: node.RHS, RHS: rhs, Op: op} break } @@ -333,6 +315,21 @@ func (p *Parser) parseUnaryExpr() (Expr, error) { return &ListLiteral{Values: tagKeys}, nil } + // handle unary negation + p.unscan() + if tok, pos, lit := p.scanIgnoreWhitespace(); tok == Not { + expr, err := p.parseUnaryExpr() + if err != nil { + return nil, err + } + + if f, ok := expr.(*FieldLiteral); ok && !fields.IsBoolean(f.Field) { + return nil, newParseError(tokstr(tok, lit), []string{"boolean field", "("}, pos, p.expr) + } + + return &NotExpr{Expr: expr}, nil + } + p.unscan() tok, pos, lit := p.scanIgnoreWhitespace() diff --git a/pkg/filter/ql/parser_test.go b/pkg/filter/ql/parser_test.go index ba50bf372..2b0ac6048 100644 --- a/pkg/filter/ql/parser_test.go +++ b/pkg/filter/ql/parser_test.go @@ -63,6 +63,10 @@ func TestParser(t *testing.T) { {expr: "ip_cidr(net.dip) = '24'", err: errors.New("ip_cidr function is undefined. Did you mean one of BASE|CIDR_CONTAINS|CONCAT|COUNT|DIR|ENTROPY|EXT|FOREACH|GET_REG_VALUE|GLOB|INDEXOF|IS_ABS|IS_MINIDUMP|LENGTH|LOWER|LTRIM|MD5|REGEX|REPLACE|RTRIM|SPLIT|SUBSTR|UNDEFINED|UPPER|VOLUME|YARA?")}, {expr: "ps.name = 'cmd.exe' and not cidr_contains(net.sip, '172.14.0.0')"}, + {expr: "ps.name = 'cmd.exe' and ps.exe not imatches '?:\\\\Windows'"}, + {expr: "ps.name not imatches 'cmd.exe' and not (ps.exe imatches '?:\\\\Windows')"}, + {expr: "ps.name not imatches 'cmd.exe' and not ps.exe", err: errors.New("ps.name not imatches 'cmd.exe' and not ps.exe\n╭──────────────────────────────────^\n|\n|\n╰─────────────────── expected boolean field, (")}, + {expr: "ps.name not imatches 'cmd.exe' and not ps.is_protected"}, {expr: `ps.envs[ProgramFiles] = 'C:\\Program Files'`}, {expr: `ps.envs imatches 'C:\\Program Files'`}, {expr: `ps.pid[1] = 'svchost.exe'`, err: errors.New("ps.pid[1] = 'svchost.exe'\n╭──────^\n|\n|\n╰─────────────────── expected field without argument")},