Skip to content

prevent nil deref when comparing to a number#34

Open
asymmetric wants to merge 1 commit intomainfrom
fix/normalize-nil-deref-on-numeric-key
Open

prevent nil deref when comparing to a number#34
asymmetric wants to merge 1 commit intomainfrom
fix/normalize-nil-deref-on-numeric-key

Conversation

@asymmetric
Copy link
Copy Markdown

@asymmetric asymmetric commented Apr 1, 2026

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.

Testing

Reproduce the specific panic that existed before this fix:

go test -run=FuzzParse/key_equals_number ./query/

Run the full fuzz harness to catch similar regressions:

go test -fuzz=FuzzParse -fuzztime=60s ./query/

@asymmetric asymmetric requested a review from draganm April 1, 2026 09:40
@asymmetric asymmetric force-pushed the fix/normalize-nil-deref-on-numeric-key branch from bb1a9a3 to 14fb598 Compare April 1, 2026 09:41
… 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.

## Testing

Reproduce the specific panic that existed before this fix:

```
go test -run=FuzzParse/key_equals_number ./query/
```

Run the full fuzz harness to catch similar regressions:

```
go test -fuzz=FuzzParse -fuzztime=60s ./query/
```

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@asymmetric asymmetric force-pushed the fix/normalize-nil-deref-on-numeric-key branch from 14fb598 to e25d60d Compare April 1, 2026 11:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant