Skip to content

[#665] Fix StackOverflowError while parsing long ACI with repetitive targets - #666

Merged
vharseko merged 2 commits into
OpenIdentityPlatform:masterfrom
vharseko:issues/665
Jul 8, 2026
Merged

vharseko merged 2 commits into
OpenIdentityPlatform:masterfrom
vharseko:issues/665

Conversation

@vharseko

@vharseko vharseko commented Jul 2, 2026

Copy link
Copy Markdown
Member

Fixes #665

Problem

Aci.decode performs a quick syntax check via Pattern.matches(aciRegex, input). The targets part of that regex,

(\(\s*(\w+)\s*(!?=)\s*"([^"]+)"\s*\)\s*)*

repeats a group with an unbounded greedy quantifier. java.util.regex matches such repetition recursively — one stack frame per repetition — so an ACI value with hundreds of repeated target rules like (targetscope="&O")(targetscope="&O")... overflows the JVM stack with StackOverflowError instead of the expected AciException. Being an Error, it escapes catch(Exception) handlers on the aci-value validation paths (AciSyntaxImpl, AciHandler, AciList) and can kill the worker thread.

Fix

  • AciTargets.targetRegex/targetsRegex: use possessive quantifiers (\s*+, \w++, (...)*+), as suggested in the issue, so the regex engine matches the repetition iteratively. Match results are unchanged (each possessive run is followed by a disjoint character class) and capture-group numbering is preserved.
  • Same-shaped list regexes hardened: oidListRegex (Aci), attrListRegex (TargetAttr), rightsRegex (Permission). New constants WORD_GROUP_POSSESSIVE and ZERO_OR_MORE_WHITESPACE_POSSESSIVE; the shared greedy constants are left untouched (ATTR_NAME intentionally not modified — it is shared with TargAttrFilterList/ParentInheritance).
  • Defense in depth: Aci.decode catches StackOverflowError and rethrows it as AciException, so a pathological ACI is treated as invalid instead of killing the thread (same approach as the GHSA-rv4q-c6mr-wxp7 fix).

Tests

New AciDecodeStackOverflowTestCase (runs decode on a bounded 256 KB stack thread so the pre-fix overflow is deterministic):

  • 10 000 repeated (targetscope="&O") targets → rejected with AciException, no StackOverflowError;
  • raw AciTargets.targetsRegex matches the same input iteratively (validates the regex fix independently of the defense-in-depth catch);
  • a valid multi-target ACI still decodes successfully.

Verified: old regex overflows a 256 KB stack on the reproducer while the new one matches iteratively, with identical match results on a corpus of valid/invalid target strings. Adjacent ACI suites pass (AciBodyTest, TargetTestCase, TargetAttrTestCase, TargAttrFiltersTestCase, ExtOpTestCase, TargetControlTestCase, EnumRightTest — 134 tests, 0 failures).

…OpenIdentityPlatform#665)

Aci.decode's quick syntax check ran Pattern.matches(aciRegex, input), whose
targets part ((\(\s*(\w+)\s*(!?=)\s*"([^"]+)"\s*\)\s*)*) repeats a group with
an unbounded greedy quantifier. java.util.regex matches such repetition
recursively - one stack frame per repetition - so an ACI value with hundreds
of repeated target rules like (targetscope="&O")... overflowed the JVM stack
with a StackOverflowError instead of the expected AciException. Being an
Error, it escaped catch(Exception) handlers on the aci-value validation paths
(AciSyntaxImpl, AciHandler, AciList) and could kill the worker thread.

- AciTargets.targetRegex/targetsRegex: use possessive quantifiers (\s*+,
  \w++, (...)*+) so the regex engine matches the repetition iteratively;
  match results are unchanged (each possessive run is followed by a disjoint
  character class) and capture-group numbering is preserved.
- Harden the same-shaped list regexes: oidListRegex (Aci), attrListRegex
  (TargetAttr), rightsRegex (Permission). New Aci constants
  WORD_GROUP_POSSESSIVE and ZERO_OR_MORE_WHITESPACE_POSSESSIVE; the shared
  greedy constants are left untouched.
- Aci.decode: defense in depth - catch StackOverflowError and rethrow as
  AciException so a pathological ACI is treated as invalid instead of
  killing the thread (same approach as GHSA-rv4q-c6mr-wxp7).
- Add AciDecodeStackOverflowTestCase: bounded-stack (256 KB) regression
  tests for the decode path and the raw targets regex, plus a valid
  multi-target decode check.
@vharseko vharseko added the bug label Jul 2, 2026
@vharseko
vharseko requested a review from maximthomas July 2, 2026 09:32
…or cause, test malformed target

- Aci.decode: pass the caught StackOverflowError as the cause of the
  rethrown AciException to ease diagnostics if the fallback ever fires.
- AciDecodeStackOverflowTestCase: add malformedTargetStillRejected to
  verify the possessive target regex still rejects invalid target syntax.
@vharseko vharseko changed the title Fix StackOverflowError while parsing long ACI with repetitive targets [#665] Fix StackOverflowError while parsing long ACI with repetitive targets Jul 3, 2026
@vharseko vharseko added the ACI Access Control Instructions subsystem label Jul 6, 2026
@vharseko
vharseko merged commit 019b2c6 into OpenIdentityPlatform:master Jul 8, 2026
33 of 34 checks passed
@vharseko
vharseko deleted the issues/665 branch July 8, 2026 16:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ACI Access Control Instructions subsystem bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

StackOverflowError while parsing long ACI with repetitive targets

2 participants