Skip to content

Commit 14fb598

Browse files
asymmetricclaude
andcommitted
fix: prevent nil deref in Normalize when $key/$owner/$creator is compared to a number
The five `Normalize()` methods on `Equality`, `LessThan`, `LessOrEqualThan`, `GreaterThan`, and `GreaterOrEqualThan` all assumed that when the variable is `$key`, `$owner`, or `$creator`, `Value.String` is non-nil. The grammar allows a bare number (e.g. `$key = 0`) to parse successfully, leaving `Value.String` nil and causing a panic on `strings.ToLower(*e.Value.String)`. Guard each site with a nil check; if the value is a number there is nothing to lowercase so we return `e` unchanged. Also adds the fuzz corpus entry and the `FuzzParse` harness that surfaced this. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 652b0b2 commit 14fb598

3 files changed

Lines changed: 114 additions & 0 deletions

File tree

query/language_fuzz_test.go

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
package query
2+
3+
import (
4+
"testing"
5+
)
6+
7+
// FuzzParse exercises the query DSL parser and normalizer with arbitrary input.
8+
// The goal is to ensure Parse never panics regardless of what a caller sends.
9+
//
10+
// Notable panic sites in normalize.go that the fuzzer can reach:
11+
// - Expression.invert(): "This should never happen!" (via NOT-paren paths)
12+
// - EqualExpr.Normalize(): "Called EqualExpr::Normalize on a paren, this is a bug!"
13+
// - EqualExpr.Normalize(): "This should not happen!"
14+
// - EqualExpr.invert(): "This should not happen!"
15+
func FuzzParse(f *testing.F) {
16+
seeds := []string{
17+
// match-all shortcuts
18+
"$all",
19+
"*",
20+
21+
// equality
22+
`name = "value"`,
23+
`name != "value"`,
24+
`$key = 0xabcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890`,
25+
`$owner = 0x1234567890abcdef1234567890abcdef12345678`,
26+
`$creator = 0x1234567890abcdef1234567890abcdef12345678`,
27+
`$expiration = 1000`,
28+
`$sequence = 42`,
29+
30+
// comparisons (numeric and string)
31+
`count > 5`,
32+
`count >= 5`,
33+
`count < 5`,
34+
`count <= 5`,
35+
`name > "bar"`,
36+
`name >= "bar"`,
37+
38+
// glob
39+
`name ~ "foo*"`,
40+
`name !~ "foo*"`,
41+
`name glob "foo*"`,
42+
`name not glob "foo*"`,
43+
44+
// inclusion
45+
`name IN ("a" "b" "c")`,
46+
`name NOT IN ("a" "b")`,
47+
`count IN (1 2 3)`,
48+
`count NOT IN (1 2 3)`,
49+
50+
// boolean combinations
51+
`name = "x" || other = "y"`,
52+
`name = "x" && other = "y"`,
53+
`name = "x" OR other = "y"`,
54+
`name = "x" AND other = "y"`,
55+
`name = "x" or other = "y"`,
56+
`name = "x" and other = "y"`,
57+
58+
// parenthesised groups
59+
`(name = "x")`,
60+
`(name = "x" && other = "y")`,
61+
`(name = "x" || other = "y") && third = "z"`,
62+
`name = "a" && (b = "c" || d = "e")`,
63+
64+
// NOT-paren (exercises the invert() path in normalize.go)
65+
`!(name = "value")`,
66+
`not(name = "value")`,
67+
`NOT(name = "value")`,
68+
`!(name = "x" && other = "y")`,
69+
`!(name = "x" || other = "y")`,
70+
`!(!(name = "value"))`,
71+
`!(name ~ "foo*")`,
72+
`!(name IN ("a" "b"))`,
73+
`!(count > 5)`,
74+
75+
// deeply nested
76+
`((name = "x"))`,
77+
`!((name = "x" || y = "z") && w = "v")`,
78+
`(a = "1" || b = "2") && (c = "3" || d = "4")`,
79+
80+
// empty / near-empty
81+
"",
82+
}
83+
84+
for _, s := range seeds {
85+
f.Add(s)
86+
}
87+
88+
f.Fuzz(func(t *testing.T, input string) {
89+
// Must not panic. Errors are fine — they represent rejected queries.
90+
defer func() {
91+
if r := recover(); r != nil {
92+
t.Errorf("Parse panicked on input %q: %v", input, r)
93+
}
94+
}()
95+
_, _ = Parse(input)
96+
})
97+
}

query/normalize.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -309,6 +309,9 @@ func (e *Glob) invert() *Glob {
309309
func (e *LessThan) Normalize() *LessThan {
310310
switch e.Var {
311311
case KeyAttributeKey, OwnerAttributeKey, CreatorAttributeKey:
312+
if e.Value.String == nil {
313+
return e
314+
}
312315
val := strings.ToLower(*e.Value.String)
313316
return &LessThan{
314317
Var: e.Var,
@@ -331,6 +334,9 @@ func (e *LessThan) invert() *GreaterOrEqualThan {
331334
func (e *LessOrEqualThan) Normalize() *LessOrEqualThan {
332335
switch e.Var {
333336
case KeyAttributeKey, OwnerAttributeKey, CreatorAttributeKey:
337+
if e.Value.String == nil {
338+
return e
339+
}
334340
val := strings.ToLower(*e.Value.String)
335341
return &LessOrEqualThan{
336342
Var: e.Var,
@@ -353,6 +359,9 @@ func (e *LessOrEqualThan) invert() *GreaterThan {
353359
func (e *GreaterThan) Normalize() *GreaterThan {
354360
switch e.Var {
355361
case KeyAttributeKey, OwnerAttributeKey, CreatorAttributeKey:
362+
if e.Value.String == nil {
363+
return e
364+
}
356365
val := strings.ToLower(*e.Value.String)
357366
return &GreaterThan{
358367
Var: e.Var,
@@ -375,6 +384,9 @@ func (e *GreaterThan) invert() *LessOrEqualThan {
375384
func (e *GreaterOrEqualThan) Normalize() *GreaterOrEqualThan {
376385
switch e.Var {
377386
case KeyAttributeKey, OwnerAttributeKey, CreatorAttributeKey:
387+
if e.Value.String == nil {
388+
return e
389+
}
378390
val := strings.ToLower(*e.Value.String)
379391
return &GreaterOrEqualThan{
380392
Var: e.Var,
@@ -397,6 +409,9 @@ func (e *GreaterOrEqualThan) invert() *LessThan {
397409
func (e *Equality) Normalize() *Equality {
398410
switch e.Var {
399411
case KeyAttributeKey, OwnerAttributeKey, CreatorAttributeKey:
412+
if e.Value.String == nil {
413+
return e
414+
}
400415
val := strings.ToLower(*e.Value.String)
401416
return &Equality{
402417
Var: e.Var,
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
go test fuzz v1
2+
string("$key =0")

0 commit comments

Comments
 (0)