Skip to content

Limit XDR nesting depth when decoding contract spec/meta sections - #2668

Merged
fnando merged 2 commits into
mainfrom
bound-xdr-decode-depth-contract-spec
Aug 4, 2026
Merged

Limit XDR nesting depth when decoding contract spec/meta sections#2668
fnando merged 2 commits into
mainfrom
bound-xdr-decode-depth-contract-spec

Conversation

@fnando

@fnando fnando commented Aug 4, 2026

Copy link
Copy Markdown
Member

What

Decode a contract's contractenvmetav0, contractmetav0, and contractspecv0 sections with an explicit XDR depth limit (SPEC_XDR_DEPTH_LIMIT = 500) instead of Limits::none().

Why

These sections come from attacker-authored contract WASM, and ScSpecTypeDef is a recursive XDR type. Decoding with Limits::none() (depth u32::MAX) lets a deeply-nested type definition recurse until it exhausts the stack and aborts the process. Capping the depth turns that crash into a clean DepthLimitExceeded error. The value 500 matches soroban-env-host's DEFAULT_XDR_RW_LIMITS, so any spec the network would accept still decodes, while deeper input is rejected. Tests cover both a deeply-nested spec that must be rejected and a normally-nested spec that must still decode.

Known limitations

N/A

Copilot AI balanced review requested due to automatic review settings August 4, 2026 17:10
@github-project-automation github-project-automation Bot moved this to Backlog (Not Ready) in DevX Aug 4, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Limits XDR decoding depth for attacker-controlled contract metadata and specs, preventing stack exhaustion.

Changes:

  • Adds a depth limit of 500 to relevant XDR decoders.
  • Adds tests for deeply and normally nested specifications.

@fnando
fnando merged commit 19c395a into main Aug 4, 2026
229 of 230 checks passed
@fnando
fnando deleted the bound-xdr-decode-depth-contract-spec branch August 4, 2026 22:21
@github-project-automation github-project-automation Bot moved this from Backlog (Not Ready) to Done in DevX Aug 4, 2026
Galmanus added a commit to Galmanus/stellar-cli that referenced this pull request Sep 1, 2026
…n recursive specs

`Spec::arg_value_name` threads a `depth` counter but never checks it. A
contract whose spec contains a recursive type — e.g.
`struct TreeNode { children: Vec<TreeNode> }`, which the SDK and the
network accept — sent the resolver into unbounded recursion, overflowing
the stack on every `contract invoke` against the contract, including
`-- --help`. Anyone can deploy such a contract, so this crashed the CLI
of anyone who tried to interact with it.

UDTs are the only types resolved by name, so any cycle must pass through
the `ScType::Udt` arm; cap the expansion depth there and render the
type's name past the cap. Returning `None` instead would erase the
whole value name, since `None` propagates through the `?` in every
caller. `example_udts` already bounds example rendering the same way.

Complements stellar#2668, which capped XDR decode depth for deeply-nested
specs; a cycle through UDT name references never nests deeply in the
encoded XDR, so it needs this separate guard.

Fixes stellar#2445
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants