panic: nil-pointer dereference when parsing a dangling open parenthesis
- Dominant language
- Go
- Stars
- 53
- Forks
- 16
- PR merge metrics
- No merged PRs in 30d
Description
## Summary
`ValidateAndNormalizeLicensesWithOptions` (and any other entry point that routes through the expression parser) panics with a nil-pointer dereference when the input contains a dangling open parenthesis, instead of reporting the string as invalid.
Affected version: **v2.7.0** (latest).
## Reproduction
```go
package main
import (
"fmt"
"github.com/github/go-spdx/v2/spdxexp"
)
func main() {
// panics: runtime error: invalid memory address or nil pointer dereference
valid, invalid := spdxexp.ValidateAndNormalizeLicensesWithOptions(
[]string{"("}, spdxexp.ValidateLicensesOptions{})
fmt.Println(valid, invalid)
}
```
Any input with an unclosed trailing open paren reproduces it: `"("`, `"(("`, `"MIT OR ("`, `"( "`.
Balanced or otherwise-malformed inputs behave correctly and are returned as `invalid` (e.g. `")"`, `"(MIT"`, `"MIT)"`, `"()"`, `"(())"`).
## Panic
```
panic: runtime error: invalid memory address or nil pointer dereference
spdxexp.(*tokenStream).parseOperator(...) spdxexp/parse.go:364
spdxexp.(*tokenStream).parseParenthesizedExpression(...) spdxexp/parse.go:97
spdxexp.(*tokenStream).parseAtom(...)
spdxexp.(*tokenStream).parseAnd(...)
spdxexp.(*tokenStream).parseExpression(...)
spdxexp.(*tokenStream).parseParenthesizedExpression(...) spdxexp/parse.go:103
...
```
## Root cause
`parseOperator` dereferences the result of `peek()` without a nil check:
```go
// spdxexp/parse.go:362
func (t *tokenStream) parseOperator(operator string) *string {
token := t.peek() // returns nil at end-of-stream
if token.role == operatorToken && token.value == operator { // line 364: nil deref
...
```
`peek()` returns `nil` when the stream is exhausted:
```go
// spdxexp/parse.go:79
func (t *tokenStream) peek() *token {
if t.hasMore() {
token := t.tokens[t.index]
return &token
}
return nil
}
```
`parseParenthesizedExpression` consumes the `(` and then recurses into `parseExpression` without first checking `hasMore()`:
```go
// spdxexp/parse.go:96
func (t *tokenStream) parseParenthesizedExpression() *node {
openParen := t.parseOperator("(")
if openParen == nil {
return nil
}
expr := t.parseExpression() // line 103: recurses even when no tokens remain
...
```
So for a trailing `(`, the recursion bottoms out in `parseParenthesizedExpression → parseOperator("(")` against an empty stream, `peek()` returns `nil`, and line 364 dereferences it.
## Expected behavior
A dangling open paren is a syntax error; it should be reported via the `invalid` return (as `"(MIT"` and `"()"` already are), not panic. Callers validating untrusted license strings currently have to wrap every call in `recover()`.
## Possible fixes
- Guard `parseOperator` (and any other unguarded `peek()` caller) against a `nil` token, or
- Have `parseParenthesizedExpression` check `t.hasMore()` before recursing into `parseExpression`.
Contributor guide
Research direction
Start with spdxexp/parse.go, especially parseOperator, peek, and parseParenthesizedExpression, then reproduce the issue through ValidateAndNormalizeLicensesWithOptions using a trailing open parenthesis. Add regression coverage for the listed dangling-parenthesis inputs and verify they return invalid without panicking, while existing malformed and balanced inputs remain unchanged.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- compilers
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 78/100