Skip to content

proof: Bound deserialize against wire-length OOM - #143

Open
nervana21 wants to merge 1 commit into
mit-dci:mainfrom
nervana21:add-caps
Open

proof: Bound deserialize against wire-length OOM#143
nervana21 wants to merge 1 commit into
mit-dci:mainfrom
nervana21:add-caps

Conversation

@nervana21

@nervana21 nervana21 commented Jul 25, 2026

Copy link
Copy Markdown

Description and Notes

Proof::deserialize previously used untrusted u64 length prefixes to size Vec::with_capacity for targets and hashes. A hostile payload could claim a huge count and OOM the process before any proof bytes were validated.

This PR rejects oversized prefixes before allocation. Target and hash counts above MAX_PROOF_DESERIALIZE_COUNT ((4 * 1024³) / 32 == 1 << 27, ~4 GiB of 32-byte hashes) return ProofError::OversizedAllocation. Cap is deserialize-only. Proof::serialize and in-memory construction are unchanged.

Changelog

proof: Bound deserialize against wire-length OOM
 - reject target/hash counts above MAX_PROOF_DESERIALIZE_COUNT before allocating
 - return ProofError::OversizedAllocation for oversized length prefixes
 - add unit tests for excessive target and hash caps

To verify changes

  • cargo test test_deserialize_rejects
  • cargo test --lib

Comment thread src/proof/mod.rs Outdated
Comment thread src/lib.rs Outdated
Comment thread src/lib.rs Outdated
Comment thread src/proof/mod.rs Outdated
@Davidson-Souza

Davidson-Souza commented Aug 7, 2026

Copy link
Copy Markdown
Collaborator

I thought about this a little more. Ideally this library won't use Bitcoin specific values — we do have other consumers that aren't doing the default Bitcoin stuff we do in Floresta.

Floresta doesn't even use this deserializer internally. I like the patch, but I would just pick a big number — like 4GB — and stick with it, rather than computing Bitcoin-specific upper bounds.

Edit: and probably make this explicit in the docs.

Check target and hash counts before Vec::with_capacity so a hostile wire
prefix cannot force a huge reservation and OOM the process.
@nervana21

Copy link
Copy Markdown
Author

Updates:

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants