feat(security): add explicit XDR length guard on returned Bytes values#1057
Merged
Baskarayelu merged 1 commit intoJun 29, 2026
Conversation
Closes Remitwise-Org#894 Without a size ceiling, any contract returning a `Bytes` value forces every downstream consumer (SDK, indexer, RPC node) to allocate memory proportional to the payload — a DoS vector that is otherwise invisible at the call site. Threat model: a misbehaving or supply-chain-compromised contract that returns an arbitrarily large `ScBytes` value causes unbounded allocation and potential OOM in every consumer that deserialises the response. The guard closes this gap as a defence-in-depth control before any external audit. Changes: - remitwise-common: add `MAX_BYTES_RETURN` (8 192 bytes), typed `BytesReturnError::ReturnTooLarge` (#[contracterror]), and `guard_bytes_len()` helper. - remittance_split: add `ReturnBytesTooLarge = 28` to `RemittanceSplitError`; change `get_request_hash` return type to `Result<Bytes, RemittanceSplitError>` and apply `guard_bytes_len` before returning; propagate with `?` in `distribute_usdc_hashed`. - Tests: four new unit tests for `guard_bytes_len` (empty, at-limit, one-over, far-above); one new contract-level test confirming the 32-byte SHA-256 result always passes the guard. All 31 affected tests pass; pre-existing failures are unchanged. Performance: the guard is a single `u32` comparison — no measurable overhead on the SHA-256 hot path. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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.
Summary
Closes #894
Add an explicit XDR byte-length guard (
guard_bytes_len) onBytesvaluesreturned from public contract entry points so consumers cannot be DoS'd by an
unbounded allocation.
Threat being mitigated
DoS via unbounded
ScBytesdeserialization.The Soroban XDR
ScBytestype carries a 4-byte unsigned length prefix with nohost-enforced upper bound before the bytes are handed to the caller. Without an
explicit check on the contract side, a misbehaving or supply-chain-compromised
contract could return an arbitrarily large
Bytespayload. Every downstreamconsumer — JavaScript SDK, Horizon indexer, RPC node, third-party integration —
must allocate a buffer proportional to
lento deserialize the value. A 2 GiBlenfield would OOM almost any process.This is a defence-in-depth fix. There is no public incident. The gap is the
kind a careful auditor would flag before any external review, and closing it now
is cheaper than remediating it under fire.
What changed
remitwise-common/src/lib.rsMAX_BYTES_RETURN = 8192, typedBytesReturnError::ReturnTooLarge(#[contracterror]),guard_bytes_len()helperremittance_split/src/lib.rsReturnBytesTooLarge = 28inRemittanceSplitError;get_request_hashnow returnsResult<Bytes, RemittanceSplitError>; guard applied before returning;distribute_usdc_hashedpropagates with?remitwise-common/src/tests.rsguard_bytes_len(empty, at-limit, one-over-limit, far-above-limit)remittance_split/src/test.rstest_get_request_hash_passes_xdr_length_guard); 8 direct struct call sites updated to.unwrap()Acceptance criteria
MAX_BYTES_RETURNtest_guard_bytes_len_rejects_oversized_value— could not be written before the fix, passes after)ScBytesallocation → consumer DoS)#[contracterror](BytesReturnError::ReturnTooLarge) surfaced instead of panic or generic errorcargo build -p remittance_split --target wasm32-unknown-unknown --releaseremitwise-commonlib clippy clean:cargo clippy -p remitwise-common --lib -- -D warnings#![no_std]discipline maintained — nostd::calls introducedPerformance
guard_bytes_lenis a singleu32comparison (bytes.len() > 8192). SHA-256always produces exactly 32 bytes, so the guard exits the fast path immediately.
No measurable overhead on the hot path.
Out of scope
Pre-existing test failures and clippy warnings in the test file are not touched,
per the issue's out-of-scope guidance.