fix(#39): enforce tuple nesting depth explicitly with MAX_NESTING_DEPTH - #67
Merged
Conversation
added 2 commits
July 4, 2026 15:23
Previously, the tuple decoder and encoder recursed without limit on nested tuples (TYPE_NESTED bytes on the wire, PHP arrays in the API). A stored value consisting of hundreds of kilobytes of `\x05` bytes (within FoundationDB's 10 MB value limit) would consume the call stack until the process aborted — a denial-of-service vector when an application called `Tuple::unpack()` on bytes produced by a less-trusted writer. The fix introduces a public `Tuple::MAX_NESTING_DEPTH = 100` bound, applied symmetrically on the encode and decode paths of `encode/decodeElement` and `encode/decodeNestedTuple`, plus the encode-side helpers `findVersionstampOffset`, `countVersionstamps` and `elementHasIncompleteVersionstamp`. Any input whose deepest recursion step would exceed the limit raises `\InvalidArgumentException` immediately at the offending call, with a message identifying both the limit and the offending depth, instead of silently consuming the stack. The guard is inclusive: a payload whose deepest recursion reaches exactly `MAX_NESTING_DEPTH` is still accepted, so legitimate deeply-nested user data round-trips unchanged. Changes: - src/Tuple/Tuple.php: introduce `MAX_NESTING_DEPTH = 100`, thread an explicit `$depth` counter through the recursive helpers, and call `assertDepth()` at the top of `encodeNestedTuple` / `decodeNestedTuple` / `elementHasIncompleteVersionstamp` / `countVersionstamps` / `findVersionstampOffset` so the limit is enforced before the next recursion step consumes the stack. - tests/Unit/Tuple/TupleNestingDepthTest.php: 13 unit tests covering the constant, unpack / pack / packWithVersionstamp / hasIncompleteVersionstamp at the accepted boundary, the rejected boundary, the 300 KB PoC buffer (must reject in well under 1 s), the pack→unpack round-trip at the limit, the encode-side helpers rejecting deep arrays even when no Versionstamp is present, and a direct `decodeNestedTuple()` call through reflection to guarantee future maintainers cannot accidentally bypass the guard. - tests/Integration/TupleNestingDepthTest.php: 2 integration tests against the live 5-node FoundationDB cluster — round-trip at the exact boundary, and the 300 KB PoC payload read back through the cluster refused on read. - docs/tuple-layer.md: document `MAX_NESTING_DEPTH` and the deep-nest DoS guard. - CHANGELOG.md: entry under `[Unreleased] / Fixed` for #39. Fixes #39 Co-authored-by: Piotr Hałas <piotr.h@decodo.com>
…limit The 5-node FDB cluster enforces a 100,000-byte value limit, which the previous integration test breached with a 300,000-byte payload. Reduce to 500 \x05 bytes — still well above MAX_NESTING_DEPTH=100 so the guard fires, while staying within the FDB-side size limit.
5 tasks
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.
Summary
Fixes #39 — the tuple decoder/encoder was recursing without limit on
nested tuples, so a stored value consisting of hundreds of kilobytes
of
\x05bytes (TYPE_NESTED) consumed the call stack until theprocess aborted whenever an application unpacked bytes produced by a
less-trusted writer.
This change introduces a public
Tuple::MAX_NESTING_DEPTH = 100bound,enforced symmetrically on both the encode and decode paths. Any
payload whose deepest recursion would exceed the limit now raises
\InvalidArgumentExceptionimmediately at the offending call, with amessage identifying both the constant and the offending depth, instead
of silently consuming the stack.
The wire format is unchanged: payloads that round-trip just at the
limit continue to be accepted, so legitimate deeply-nested user data
is unaffected.
Changes
src/Tuple/Tuple.php— new publicMAX_NESTING_DEPTHconstantassertDepth()helper, threaded throughencode/decodeNestedTuple,encode/decodeElement,elementHasIncompleteVersionstamp,countVersionstampsandfindVersionstampOffset.tests/Unit/Tuple/TupleNestingDepthTest.php— 13 unit tests:constant exposure; unpack/pack/packWithVersionstamp/hasIncomplete
at the accepted and rejected boundaries; the 300 KB PoC buffer
(must reject in well under 1 s); pack→unpack round-trip at the
limit; encode-side helpers rejecting deep arrays even when no
Versionstamp is present; and a direct
decodeNestedTuple()callvia reflection so a future maintainer cannot accidentally bypass
the guard.
tests/Integration/TupleNestingDepthTest.php— 2 integrationtests against the live 5-node cluster: round-trip at the exact
boundary and the PoC payload refused on read.
docs/tuple-layer.md— new "Nesting Depth Limit" sectiondocumenting
MAX_NESTING_DEPTHand the DoS guard.CHANGELOG.md—[Unreleased] / Fixedentry for [Security] Unbounded recursion in Tuple decoder enables denial of service #39.Verification
composer lintclean (PHPCS, Rector, PHPStan).composer test:unitclean (380 tests pass).E2E Tests (5-node FDB cluster)job against the 5-node FDB cluster.
Fixes #39