Skip to content

test(policydsl): add regression tests for stack-overflow DoS (#5304) - #5527

Open
PrazwalR wants to merge 1 commit into
hyperledger:mainfrom
PrazwalR:test/policydsl-deepnest-regression
Open

test(policydsl): add regression tests for stack-overflow DoS (#5304)#5527
PrazwalR wants to merge 1 commit into
hyperledger:mainfrom
PrazwalR:test/policydsl-deepnest-regression

Conversation

@PrazwalR

Copy link
Copy Markdown

Adds regression tests for #5304 (stack overflow via deeply nested
policy DSL expressions).

Why

FromString() previously used Knetic/govaluate, whose recursive
planner had no depth/size limit and could overflow the goroutine
stack on maliciously nested AND/OR/OutOf policy strings
(confirmed: reproduces fatal error: stack overflow in
planValue -> planAccessor -> planFunction -> planPrecedenceLevel
on the old dependency).

This was already fixed on main by the govaluate -> expr-lang/expr
migration in #5480 (common/policydsl/policyparser.go), which
enforces expr's DefaultMaxNodes (1e4) at parse time. This PR adds
no production code change — only tests pinning that behavior so it
cannot silently regress:

  • deeply nested (up to 200k levels) AND/OR policies parse without
    crashing, either succeeding or failing with the expected
    "exceeds maximum allowed nodes" error
  • the node cap is confirmed structural (AST size), not a naive
    input-length check — a wide-but-shallow 2000-arg OutOf still parses
  • malformed/adversarial input (unbalanced parens, 1MB tokens, unknown
    gates) never panics

Test plan

  • go test -race -v ./common/policydsl/... — all pass
  • go vet ./common/policydsl/... — clean
  • gofmt -l common/policydsl/policyparser_deepnest_test.go — clean

Copilot AI review requested due to automatic review settings July 21, 2026 15:31
@PrazwalR
PrazwalR requested a review from a team as a code owner July 21, 2026 15:31

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

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.

2 participants