Add depth limit to xdr encoding and decoding - #2675
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a 500-level depth limit to XDR encoding and decoding to prevent stack-overflow crashes.
Changes:
- Replaces unlimited XDR operations across CLI commands.
- Applies matching limits to supporting crates.
- Updates related test fixture encoding.
Reviewed changes
Copilot reviewed 32 out of 32 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
cmd/soroban-cli/src/utils.rs |
Defines the shared limit and applies it to utility XDR operations. |
cmd/soroban-cli/src/signer/mod.rs |
Limits signer XDR encoding. |
cmd/soroban-cli/src/log/event.rs |
Limits diagnostic-event encoding. |
cmd/soroban-cli/src/key.rs |
Limits key XDR decoding. |
cmd/soroban-cli/src/config/data.rs |
Limits cached-data encoding. |
cmd/soroban-cli/src/commands/tx/xdr.rs |
Limits transaction input decoding. |
cmd/soroban-cli/src/commands/tx/update/sequence_number/next.rs |
Limits updated transaction output. |
cmd/soroban-cli/src/commands/tx/simulate.rs |
Limits simulated transaction output. |
cmd/soroban-cli/src/commands/tx/sign.rs |
Limits signed transaction output. |
cmd/soroban-cli/src/commands/tx/op/add/mod.rs |
Limits operation transaction output. |
cmd/soroban-cli/src/commands/tx/fetch/result.rs |
Limits fetched result encoding. |
cmd/soroban-cli/src/commands/tx/fetch/meta.rs |
Limits fetched metadata encoding. |
cmd/soroban-cli/src/commands/tx/fetch/envelope.rs |
Limits fetched envelope encoding. |
cmd/soroban-cli/src/commands/tx/edit.rs |
Limits transaction JSON/XDR conversion. |
cmd/soroban-cli/src/commands/tx/args.rs |
Limits transaction output encoding. |
cmd/soroban-cli/src/commands/snapshot/create.rs |
Limits archive XDR decoding. |
cmd/soroban-cli/src/commands/network/settings.rs |
Limits settings XDR output. |
cmd/soroban-cli/src/commands/ledger/entry/fetch/contract_data.rs |
Limits contract-key decoding. |
cmd/soroban-cli/src/commands/events.rs |
Limits event value and topic decoding. |
cmd/soroban-cli/src/commands/contract/upload.rs |
Limits upload-related XDR operations. |
cmd/soroban-cli/src/commands/contract/restore.rs |
Limits restore transaction output. |
cmd/soroban-cli/src/commands/contract/read.rs |
Limits contract-data output encoding. |
cmd/soroban-cli/src/commands/contract/invoke.rs |
Limits invoke transaction output. |
cmd/soroban-cli/src/commands/contract/id/wasm.rs |
Limits contract-ID preimage encoding. |
cmd/soroban-cli/src/commands/contract/extend.rs |
Limits extend transaction output. |
cmd/soroban-cli/src/commands/contract/deploy/wasm.rs |
Limits WASM deployment output. |
cmd/soroban-cli/src/commands/contract/deploy/asset.rs |
Limits asset deployment output. |
cmd/soroban-cli/src/commands/contract/build.rs |
Limits contract metadata and specification encoding. |
cmd/soroban-cli/src/assembled.rs |
Limits simulation and assembly XDR operations. |
cmd/crates/stellar-ledger/src/lib.rs |
Limits Ledger signing payload encoding. |
cmd/crates/soroban-spec-typescript/src/lib.rs |
Limits TypeScript specification encoding. |
cmd/crates/soroban-spec-tools/src/contract.rs |
Limits specification JSON encoding. |
leighmcculloch
left a comment
There was a problem hiding this comment.
This seems fine, and also relatively low risk making this change in the cli since if a problem does arise where we have set it too low then we can quickly ship a new release and the release channels are all very accessible.
See inline comments as there are some places that I think we won't see the intended affect because the rpc client crate already decodes them without a depth limit (iirc we have not added depth limits there).
Something else I wonder about is will the depth limit actually prevent stack overflow in some of the cases like claimable balances. But in any case this is an improvement.
What
Adds a depth limit to all XDR encoding and decoding sites.
Why
This also prevents stack overflow crashes if intentionally deep XDR structs are encountered.
Known limitations
None