Skip to content

Bounds-check mm_tag.depth before allocating parent blockchain_branch for v2/v3 blocks#31

Closed
MoneroOcean wants to merge 1 commit into
masterfrom
codex/fix-forknote-v2/v3-parent-block-vulnerability
Closed

Bounds-check mm_tag.depth before allocating parent blockchain_branch for v2/v3 blocks#31
MoneroOcean wants to merge 1 commit into
masterfrom
codex/fix-forknote-v2/v3-parent-block-vulnerability

Conversation

@MoneroOcean
Copy link
Copy Markdown
Owner

Motivation

  • Parsing of parent_block for BLOCK_MAJOR_VERSION_2/BLOCK_MAJOR_VERSION_3 made an untrusted tx_extra merge-mining depth reachable from callers and it was used to resize blockchain_branch without bounds, allowing OOM or exception aborts.
  • The goal is to prevent attacker-controlled large mm_tag.depth from driving an unchecked std::vector::resize while preserving valid deserialization behavior.

Description

  • Add a deserialization-time guard in src/cryptonote_basic/cryptonote_basic.h that checks mm_tag.depth against ar.remaining_bytes() / sizeof(crypto::hash) before calling PREPARE_CUSTOM_VECTOR_SERIALIZATION.
  • The check is only applied on load (!typename Archive<W>::is_saving()) and returns false for inputs that claim more branch entries than remain in the serialized stream.
  • This prevents an attacker from causing a huge allocation for b.blockchain_branch when the claimed depth cannot be satisfied by the input bytes.

Testing

  • Ran npm test in this environment, which failed before running tests due to dependency fetch error (403 Forbidden - GET https://registry.npmjs.org/bech32) so test suite could not be executed.

Codex Task

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant