Skip to content

fix(tx_extra): enforce equal vector lengths in tx_extra_master_node_register#197

Open
raw391 wants to merge 1 commit into
Beldex-Coin:devfrom
raw391:fix/tx-extra-mn-register-vector-invariant
Open

fix(tx_extra): enforce equal vector lengths in tx_extra_master_node_register#197
raw391 wants to merge 1 commit into
Beldex-Coin:devfrom
raw391:fix/tx-extra-mn-register-vector-invariant

Conversation

@raw391

@raw391 raw391 commented Jun 4, 2026

Copy link
Copy Markdown

tx_extra_master_node_register at src/cryptonote_basic/tx_extra.h:317-334 carries three parallel vectors (m_public_spend_keys, m_public_view_keys, m_portions) that are intended to be index-aligned. The serializer reads each as an independent vector with no cross-field invariant, so a malformed field with desynchronized lengths deserializes successfully and exposes consumers to out-of-bounds reads.

Two consumers index by one vectors size and read another. The block-include path at master_node_list.cpp:305-309 (reg_tx_extract_fields):

for (size_t i = 0; i < registration.m_public_spend_keys.size(); i++) {
  contributor_args.addresses.back().m_view_public_key = registration.m_public_view_keys[i];
}

validate_contributor_args (line 333) catches the size mismatch only after the loop runs.

The RPC path at core_rpc_server.cpp:711-720 (extra_extractor::operator()(tx_extra_master_node_register&)) is reachable from any otherwise valid relayable transaction that carries a malformed tx_extra_master_node_register field. tx_pool::add_tx accepts arbitrary tx.extra content on standard transfer transactions, so the tx sits in mempool until a caller queries get_transactions or get_transaction_pool with tx_extra=true. The visitor then reads past the end of one of the key vectors; outcome depends on heap layout.

Patch enforces the equal-length invariant at deserialization (matching the tx_extra_padding pattern at tx_extra.h:195-210), and adds size-equality guards at both consumers so the call sites stay safe if the invariant ever drifts.

   struct tx_extra_master_node_register
   {
     ...
     BEGIN_SERIALIZE()
       FIELD(m_public_spend_keys)
       FIELD(m_public_view_keys)
       FIELD(m_portions_for_operator)
       FIELD(m_portions)
       FIELD(m_expiration_timestamp)
       FIELD(m_master_node_signature)
+      if (Archive::is_deserializer)
+      {
+        if (m_public_spend_keys.size() != m_public_view_keys.size())
+          throw std::invalid_argument{"tx_extra_master_node_register: spend/view key count mismatch"};
+        if (m_public_spend_keys.size() != m_portions.size())
+          throw std::invalid_argument{"tx_extra_master_node_register: spend/portions count mismatch"};
+      }
     END_SERIALIZE()
   };

…egister

tx_extra_master_node_register carries three parallel vectors
(m_public_spend_keys, m_public_view_keys, m_portions) that are
intended to be index-aligned. The serializer reads each as an
independent vector with no cross-field invariant, so a malformed
field with desynchronized lengths deserializes successfully and
exposes consumers to out-of-bounds reads.

Two consumers index by one vector size and read another:
master_node_list.cpp:305-309 (block-include path) and
core_rpc_server.cpp:711-720 (mempool-staged + RPC tx_extra=true).

Enforces the equal-length invariant in the deserializer, matching
the tx_extra_padding pattern at tx_extra.h:195-210. Adds defensive
guards at both consumers so the call sites stay safe if the
invariant drifts in a future change.
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