Skip to content

Harden filter parser/lexer against malicious search expressions (DoS)#154

Open
MrEchoFi wants to merge 2 commits into
dstotijn:mainfrom
MrEchoFi:fix/filter-parser-dos
Open

Harden filter parser/lexer against malicious search expressions (DoS)#154
MrEchoFi wants to merge 2 commits into
dstotijn:mainfrom
MrEchoFi:fix/filter-parser-dos

Conversation

@MrEchoFi

Copy link
Copy Markdown

Fixes #153 .

filter.ParseQuery is reachable from the unauthenticated GraphQL API via
search/filter fields (setHttpRequestLogFilter, setSenderRequestFilter,
updateInterceptSettings). This PR fixes two denial-of-service issues in
pkg/filter.

1. Stack overflow → whole-process crash (critical)

The recursive-descent parser had no depth limit. Deeply nested input — many
( or repeated NOT — recurses through parseExpression until the goroutine
stack is exhausted. In Go a stack overflow is a fatal runtime error: it
cannot be caught by recover(), so gqlgen's panic-recovery middleware does
not contain it and the entire process crashes (proxy, admin API and all
in-flight intercepts). The parse happens before the "no active project" check,
so no project/auth/setup is required.

Fix: add a maxParseDepth (256) guard in parseExpression, the single
recursion point reached by grouped/prefix/infix sub-expression parsing.
Legitimate, human-written queries nest far below this.

2. Lexer goroutine leak (high)

NewLexer runs a goroutine that emits tokens on an unbuffered channel. When
ParseQuery returned early on a parse error it stopped draining the channel,
leaving that goroutine blocked on its send forever. Every malformed query
leaked one goroutine → gradual goroutine/memory exhaustion, also remotely
triggerable.

Fix: make the lexer cancellable (a done channel + Stop()); ParseQuery
defers Stop() so the goroutine is released on every return path.

How to verify

Crash on current main:

cat > pkg/filter/poc_test.go <<'EOF'
package filter
import ("strings";"testing")
func TestPoC(t *testing.T){ ParseQuery(strings.Repeat("(", 2_000_000)) }
EOF
go test ./pkg/filter/ -run TestPoC   # => fatal error: stack overflow
rm pkg/filter/poc_test.go

With this PR:

go test ./pkg/filter/ -run 'Depth|Leak|Nesting' -v   # all PASS, no crash
go test ./pkg/filter/                                  # full suite green

Tests added

  • TestParseQueryDepthLimit — 1M-deep ( / NOT / nested-group inputs now
    return ErrMaxDepthExceeded instead of crashing.
  • TestParseQueryReasonableNestingStillWorks — legitimate nesting still parses.
  • Goroutine-leak regression test.

No behavior change for valid expressions; no public API change.

MrEchoFi added 2 commits June 14, 2026 15:52
NewLexer spawns a goroutine that emits tokens on an unbuffered channel.
When ParseQuery returned early on a parse error, it stopped reading
tokens, leaving the lexer goroutine blocked forever on a channel send.

Because ParseQuery is reachable from the unauthenticated GraphQL API via
search expressions (httpRequestLogFilter / senderRequestFilter), each
malformed query leaked one goroutine permanently, allowing remote
goroutine/memory exhaustion (DoS).

Make the lexer cancellable via a `done` channel and a `Stop` method.
ParseQuery now defers `Stop`, releasing the goroutine on every return
path. Adds a regression test asserting no goroutines leak across many
malformed queries.
The recursive-descent parser had no depth limit. Deeply nested input
(e.g. many "(" or repeated "NOT" prefixes) recursed until the goroutine
stack was exhausted. In Go a stack overflow is a *fatal* runtime error:
it cannot be caught by recover(), so it crashes the entire process.

ParseQuery is reachable from the unauthenticated GraphQL API via search
expressions, so a single small request (a few hundred KB of "(") would
crash the whole Hetty instance — proxy, admin API and all in-flight
intercepts. gqlgen's panic recovery does not help, because this is not a
recoverable panic.

Add a maxParseDepth (256) guard in parseExpression, the single recursion
point reached by grouped, prefix and infix sub-expression parsing.
Legitimate human-written search queries nest far below this limit.
@MrEchoFi MrEchoFi force-pushed the fix/filter-parser-dos branch from 59a19ec to fad7c41 Compare June 14, 2026 22:07
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.

Unauthenticated remote DoS: a single GraphQL search expression crashes the whole process (filter parser stack overflow)

1 participant