Drop CSS declarations that nest functions deeper than sixteen - #508
Merged
Merged
Conversation
CssGrammar.parsePropertyValue recurses once per function, with no bound
on the depth, so a style attribute that nested functions without bound
overflowed the thread's stack and the StackOverflowError escaped from
PolicyFactory.sanitize(): about 60 KB of "rgb(" on a default-sized
stack, about 5 KB on a 256 KB one, on JDK 11, 17, 21 and 25, with the
prepackaged Sanitizers.STYLES.
Before a declaration's value is parsed, scan it for the deepest function
nesting; past MAX_FUNCTION_DEPTH, sixteen, skip to the end of the
declaration without handing any of it to the handler. The scan stops
exactly where the value parse would, at the first semicolon inside no
function, so that no token is read twice: a scan that ran on to the
matching bracket and seeked back was quadratic on "a:(;a:)" repeated.
The lexer pairs every bracket, so a bit per depth is enough to tell a
function's close from a bare bracket's. Declarations before and after
the deep one are kept, one at the limit parses as it always has, and
bare brackets, which are punctuation, do not count. Nothing in
CssSchema has a use for anything near that depth.
Regression tests at the limit, one past it, and at two hundred thousand
levels, with and without the closing brackets, at the StylingPolicy
level and through Sanitizers.STYLES.and(Sanitizers.BLOCKS), plus the
rescan shape under a timeout. With the guard disabled, two of them die
with StackOverflowError and two fail on output.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
For every close bracket, CssTokens.Lexer.closeBracket walked the stack
of open brackets looking for the partner, and walked all of it when
there was none: a run of unmatched closes after a run of opens took time
quadratic in the input, twenty seconds for 320 KB of "(" then "]",
through sanitize() with Sanitizers.STYLES. Found by the review of the
nesting fix, and of the same class.
Keep, per close bracket character, a count of the open brackets it
would close, and drop a close with no partner without the walk. A
close that has a partner still pops everything above it, but each open
is popped once, so that is linear. The normalized output is unchanged.
Tests pin the normalization of orphaned and mismatched closes, and put
four hundred thousand of each shape under a timeout, in CssTokensTest
and through StylingPolicy.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The CSS grammar's unbounded recursion was a class of bug the guidance did not name: it spoke of bypasses, not of input that crashes or hangs the sanitizer. Name it as an invariant, ask for a hostile-size test wherever code walks the input recursively or keeps state per token, list it with the regression targets, and say how this file is to grow when the next mistake shows a rule was missing. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The depth scan recorded, per bracket depth, whether the open bracket is
a function, in a BitSet. BitSet.clear(int) looks back from the highest
word in use for the next non-zero word, and the marks are indexed by
total bracket depth, which the function limit does not bound. A value
that opens deep with bare brackets, then sets and clears its top mark
over and over ("(" x d then "rgb()()" x k) paid d/64 per repeat: 162,
274, 850 and 3,165 ms at 1, 2, 4 and 8 MB. Found by Astra's review.
Use a boolean array grown by doubling and allocated per declaration, so
every open and close costs constant time and no state crosses from one
declaration to the next. The same shape now takes 116, 94, 140 and
304 ms; the other fourteen hostile shapes are unchanged, and 500,000
random inputs give the same output as before.
The timed test gains this shape at 4 MB. Its 20 s budget guards only
a gross regression: the quadratic constant here is small enough that
the old code passed it in under a second, and the scaling above is the
evidence.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Member
Author
|
Review follow-up (Astra's finding at 82e9b10):
The timed test gains the shape at 4 MB under the suite's 20 s budget. That budget guards only a gross regression: the old code passed it in under a second, so the scaling numbers above are the evidence, not the test. |
jmanico
added a commit
that referenced
this pull request
Sep 25, 2026
The javadoc added in cf9567f said the scan's over-count, after a semicolon inside a function, drops only values no browser keeps. Not so: CSS if() separates its branches with semicolons, so "margin: 1px if(media(print): 2px; else: calc(...))" is valid CSS. The scan drops it whole once the else branch nests sixteen calc() deep, where the parse before #508 kept "margin:1px" at any depth. Found by Sol-6's review. Say that it can lose valid CSS, that the loss is conservative, and that only a value nested past the limit is affected. No code change. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
jmanico
added a commit
that referenced
this pull request
Sep 25, 2026
Take the review of #508: depth scan and lexer cleanups
|
🎉 This issue has been resolved in |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What broke
CssGrammar.parsePropertyValuerecurses once per CSS function with no bound on the depth. Astyleattribute that nests functions without bound, such asrgb(rgb(rgb(...)))a few thousand times, overflows the thread's stack, and theStackOverflowErrorescapes fromPolicyFactory.sanitize(). Reproduced with the prepackagedSanitizers.STYLES.and(Sanitizers.BLOCKS)on JDK 11, 17, 21 and 25: about 60 KB ofrgb(on a default-sized stack, about 5 KB with-Xss256k. CWE-674, uncontrolled recursion; tracked in draft advisory GHSA-x6fc-6qh4-g3wr.The review of the fix also found a pre-existing quadratic path in the CSS lexer: a run of close brackets with no open partner after a run of opens walked the whole open-bracket stack per close. 320 KB of
(then]took twenty seconds throughsanitize().What changed
skipIfNestedTooDeeplyscans it for the deepest function nesting. PastMAX_FUNCTION_DEPTH, sixteen, it skips to the end of the declaration without handing any of it to the handler, so the declaration is dropped whole. The scan stops exactly where the value parser stops, at the first semicolon inside no function, so no token is read twice (a scan that ran on to the matching bracket and seeked back was itself quadratic ona:(;a:)repeated). Declarations before and after the deep one are kept, one at the limit parses exactly as before, and bare brackets, which are punctuation, do not count. Nothing inCssSchemahas a use for anything near sixteen levels.CssTokens.Lexerkeeps a count of open brackets per close character and drops an unmatched close in constant time. Normalized output is unchanged.AGENTS.mdgains the denial-of-service invariant, a hostile-size test rule, and a note on how that file should grow when a mistake shows a rule was missing.Verification
./mvnw clean verifypasses on JDK 11, 17, 21 and 25 (710 library tests, 7 in the JPMS consumer check).StackOverflowErrorand two fail on output.calc(deep, wide function lists) are linear at 50k, 100k and 200k units with-Xss256k; the largest, 21 MB, takes 0.6 s.mainin two cases, both nested past the limit and both fail closed (dropped).!important) give output identical tomain.Release
change_log.mdhas theNext releaseentry the release workflow needs.docs/vulnerabilities.md.🤖 Generated with Claude Code