github / github/go-spdx

panic: nil-pointer dereference when parsing a dangling open parenthesis

Open Beginner friendly
#158 0 comments 0 reactions 0 assignees View on GitHub
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.