kornys opened a new pull request, #772:
URL: https://github.com/apache/commons-text/pull/772
This ports the recursion bound applied to the deprecated commons-lang3
StrSubstitutor (apache/commons-lang c8dc3121) over to StringSubstitutor, as
suggested in the security-report thread ("Then please provide a PR on GitHub").
**Problem**
StringSubstitutor's checkCyclicSubstitution() only rejects a variable
already on the current substitution stack. It does not bound:
* acyclic fan-out — each of N references expanding to N more (a
"billion-laughs" shape). No variable ever repeats on the stack, so the cycle
check never fires.
* deep nesting — ${v0} → ${v1} → … recurses once per level toward
StackOverflowError (also reachable via nested variable names when
setEnableSubstitutionInVariables(true) is set).
A crafted variable map (where the substitution values are
attacker-controlled) can therefore drive interpolation into exponential output
growth (CPU/memory exhaustion) or a stack overflow.
**Fix**
Two budgets per top-level substitution, matching the lang3 fix:
* MAX_SUBSTITUTION_DEPTH = 256 — nesting-depth cap
* MAX_SUBSTITUTION_LENGTH = 16 MiB — total emitted-characters cap
Both raise IllegalStateException when exceeded. substitute(...) is split
into a small budget-enforcing wrapper plus the existing recursive body
(substituteRecursive), so the counters cover value recursion and nested-name
resolution alike and reset per top-level call — a budget-exceeded failure does
not poison the next replace().
No public API change; behavior is unchanged for any input under the budgets.
**Tests**
Two regression tests added to StringSubstitutorTest (deep nesting → depth
budget; exponential fan-out → size budget). The full StringSubstitutorTest
suite passes (81 tests).
Before you push a pull request, review this list:
- [x] Read the [contribution guidelines](CONTRIBUTING.md) for this project.
- [x] Read the [ASF Generative Tooling
Guidance](https://www.apache.org/legal/generative-tooling.html) if you use
Artificial Intelligence (AI).
- [x] I used AI to create any part of, or all of, this pull request. Which
AI tool was used to create this pull request, and to what extent did it
contribute?
Yes. I used Claude (Anthropic). It is a port of the maintainer's own
commons-lang3 fix (c8dc3121) to StringSubstitutor; Claude wrote the two unit
tests, and drafted this description under my direction. I reviewed the result,
confirmed the tests fail without the runtime change, and I take responsibility
for the contribution.
- [x] Run a successful build using the default
[Maven](https://maven.apache.org/) goal with `mvn`; that's `mvn` on the command
line by itself.
- [x] Write unit tests that match behavioral changes, where the tests fail
if the changes to the runtime are not applied. This may not always be possible,
but it is a best practice.
- [x] Write a pull request description that is detailed enough to understand
what the pull request does, how, and why.
- [x] Each commit in the pull request should have a meaningful subject line
and body. Note that a maintainer may squash commits during the merge process.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]