Repository navigation
Bound the nesting depth of every recursive XDR type - #197
Merged
Merged
Conversation
Closes a gap in the binary XDR decoder. Before this change, only `XdrSorobanDelegateSignature::decode` counted its nesting against the buffer's recursion cap. The twelve other XDR types that contain themselves, among them `XdrSCVal` through vectors and maps, `XdrClaimPredicate` and `XdrSCSpecTypeDef`, decoded without a bound. A value nested deeper than the C stack allows decoded, and releasing the decoded object graph ended the process with a segmentation fault. With Xdebug loaded, the decode ended in an `Error`, which a `catch (Exception $e)` clause does not catch. The network reads XDR to a depth of 500 levels, so such bytes come from outside the network. Examples are a transaction envelope, an authorization entry or a contract spec that an application receives for signing or display. This change: - Makes the generator emit a depth guard at the entry of every decoder on a cycle of the XDR type graph, unions included, so fields, optionals, arrays and union arms share one budget. The guard is the buffer's counter of active recursive decodes, checked before the increment and released in `finally`, with a cap of 500 (`XdrBuffer::DECODE_DEPTH_LIMIT`), the network's XDR depth. The network counts every nested type and the guard counts only recursive ones, so no value the network accepts reaches the cap. - Keeps the delegate chain's limit of 128 signatures (`XdrBuffer::RECURSION_LIMIT`) and the message that names it. The delegate decoder counts its chain per buffer beside the shared budget, so a chain of 128 delegate signatures decodes and the decoder rejects a chain of 129, as before. - Checks the cap before the increment, so a buffer keeps its full budget after a failed decode. - Adds a generator test that computes the cycles of the generated decoders and checks that exactly these thirteen types carry one balanced guard each. - Adds decoder tests for the boundaries at 500 and 501 levels for vectors, maps, claim predicates and authorized invocations, where a map entry and its value count one level each. Further tests cover the delegate boundary at 128 and 129 and a chain of delegates and values on one budget. Two more tests check sibling breadth, which consumes no depth, and the counter after a success and after a failure on the same buffer. - Moves the TxRep entry of the CHANGELOG from the 1.17.0 section to Unreleased. The unit suite, PHPStan, the generated suite, the generator tests and the XDR validation pass, and a second regeneration changes no file. Closes #196
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #197 +/- ##
=========================================
Coverage 93.45% 93.46%
- Complexity 21602 21603 +1
=========================================
Files 1006 1006
Lines 52362 52392 +30
=========================================
+ Hits 48937 48967 +30
Misses 3425 3425
🚀 New features to boost your workflow:
|
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
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.
Closes a gap in the binary XDR decoder. Before this change, only
XdrSorobanDelegateSignature::decodecounted its nesting against the buffer's recursion cap. The twelve other XDR types that contain themselves, among themXdrSCValthrough vectors and maps,XdrClaimPredicateandXdrSCSpecTypeDef, decoded without a bound. A value nested deeper than the C stack allows decoded, and releasing the decoded object graph ended the process with a segmentation fault. With Xdebug loaded, the decode ended in anError, which acatch (Exception $e)clause does not catch. The network reads XDR to a depth of 500 levels, so such bytes come from outside the network. Examples are a transaction envelope, an authorization entry or a contract spec that an application receives for signing or display.This change:
finally, with a cap of 500 (XdrBuffer::DECODE_DEPTH_LIMIT), the network's XDR depth. The network counts every nested type and the guard counts only recursive ones, so no value the network accepts reaches the cap.XdrBuffer::RECURSION_LIMIT) and the message that names it. The delegate decoder counts its chain per buffer beside the shared budget, so a chain of 128 delegate signatures decodes and the decoder rejects a chain of 129, as before.The unit suite, PHPStan, the generated suite, the generator tests and the XDR validation pass, and a second regeneration changes no file.
Closes #196