From 1d5bf8cc8e1fced61f1526f85dc39e34d5e38349 Mon Sep 17 00:00:00 2001 From: j-berman Date: Fri, 7 Aug 2026 12:21:00 -0700 Subject: [PATCH] cryptonote_basic: parse_and_validate_tx param to check max size Simple API to check max blob size before parsing. This is useful when reading blobs from untrusted sources. @selsta pointed out that coinbase txs can technically be larger than get_max_tx_size(), otherwise we could enforce it on all txs. (cherry picked from commit b66e2227c2f937a3e09dd62ed770b0667bff0779) --- .../cryptonote_format_utils.cpp | 22 ++++++++++++++----- .../cryptonote_format_utils.h | 11 +++++----- src/cryptonote_core/blockchain.cpp | 2 +- src/cryptonote_core/cryptonote_core.cpp | 4 ++-- src/cryptonote_core/tx_sanity_check.cpp | 2 +- .../cryptonote_protocol_handler.inl | 13 ++++------- src/wallet/wallet2.cpp | 2 +- 7 files changed, 31 insertions(+), 25 deletions(-) diff --git a/src/cryptonote_basic/cryptonote_format_utils.cpp b/src/cryptonote_basic/cryptonote_format_utils.cpp index d4a13e581..a8d4b8498 100644 --- a/src/cryptonote_basic/cryptonote_format_utils.cpp +++ b/src/cryptonote_basic/cryptonote_format_utils.cpp @@ -123,6 +123,12 @@ namespace cryptonote return bp_clawback; } + bool passes_max_size_check(const bool max_size_check, const blobdata_ref &tx_blob) + { + if (!max_size_check) + return true; + return tx_blob.size() <= get_max_tx_size(); + } //--------------------------------------------------------------- } @@ -242,8 +248,9 @@ namespace cryptonote return true; } //--------------------------------------------------------------- - bool parse_and_validate_tx_from_blob(const blobdata_ref& tx_blob, transaction& tx) + bool parse_and_validate_tx_from_blob(const blobdata_ref& tx_blob, transaction& tx, const bool max_size_check) { + CHECK_AND_ASSERT_MES(passes_max_size_check(max_size_check, tx_blob), false, "Tx blob too big"); binary_archive ba{epee::strspan(tx_blob)}; bool r = ::serialization::serialize(ba, tx); CHECK_AND_ASSERT_MES(r, false, "Failed to parse transaction from blob"); @@ -253,8 +260,9 @@ namespace cryptonote return true; } //--------------------------------------------------------------- - bool parse_and_validate_tx_base_from_blob(const blobdata_ref& tx_blob, transaction& tx, const bool require_eof) + bool parse_and_validate_tx_base_from_blob(const blobdata_ref& tx_blob, transaction& tx, const bool max_size_check, const bool require_eof) { + CHECK_AND_ASSERT_MES(passes_max_size_check(max_size_check, tx_blob), false, "Tx blob too big"); binary_archive ba{epee::strspan(tx_blob)}; bool r = tx.serialize_base(ba); CHECK_AND_ASSERT_MES(r, false, "Failed to parse transaction from blob"); @@ -264,16 +272,18 @@ namespace cryptonote return true; } //--------------------------------------------------------------- - bool parse_and_validate_tx_prefix_from_blob(const blobdata_ref& tx_blob, transaction_prefix& tx) + bool parse_and_validate_tx_prefix_from_blob(const blobdata_ref& tx_blob, transaction_prefix& tx, const bool max_size_check) { + CHECK_AND_ASSERT_MES(passes_max_size_check(max_size_check, tx_blob), false, "Tx blob too big"); binary_archive ba{epee::strspan(tx_blob)}; bool r = ::serialization::serialize_noeof(ba, tx); CHECK_AND_ASSERT_MES(r, false, "Failed to parse transaction prefix from blob"); return true; } //--------------------------------------------------------------- - bool parse_and_validate_tx_from_blob(const blobdata_ref& tx_blob, transaction& tx, crypto::hash& tx_hash) + bool parse_and_validate_tx_from_blob(const blobdata_ref& tx_blob, transaction& tx, crypto::hash& tx_hash, const bool max_size_check) { + CHECK_AND_ASSERT_MES(passes_max_size_check(max_size_check, tx_blob), false, "Tx blob too big"); binary_archive ba{epee::strspan(tx_blob)}; bool r = ::serialization::serialize(ba, tx); CHECK_AND_ASSERT_MES(r, false, "Failed to parse transaction from blob"); @@ -285,9 +295,9 @@ namespace cryptonote return get_transaction_hash(tx, tx_hash); } //--------------------------------------------------------------- - bool parse_and_validate_tx_from_blob(const blobdata_ref& tx_blob, transaction& tx, crypto::hash& tx_hash, crypto::hash& tx_prefix_hash) + bool parse_and_validate_tx_from_blob(const blobdata_ref& tx_blob, transaction& tx, crypto::hash& tx_hash, crypto::hash& tx_prefix_hash, const bool max_size_check) { - if (!parse_and_validate_tx_from_blob(tx_blob, tx, tx_hash)) + if (!parse_and_validate_tx_from_blob(tx_blob, tx, tx_hash, max_size_check)) return false; get_transaction_prefix_hash(tx, tx_prefix_hash); return true; diff --git a/src/cryptonote_basic/cryptonote_format_utils.h b/src/cryptonote_basic/cryptonote_format_utils.h index f0f25575f..8ed3ec008 100644 --- a/src/cryptonote_basic/cryptonote_format_utils.h +++ b/src/cryptonote_basic/cryptonote_format_utils.h @@ -52,11 +52,12 @@ namespace cryptonote crypto::hash get_transaction_prefix_hash(const transaction_prefix& tx, hw::device &hwdev); void get_transaction_prefix_hash(const transaction_prefix& tx, crypto::hash& h); crypto::hash get_transaction_prefix_hash(const transaction_prefix& tx); - bool parse_and_validate_tx_prefix_from_blob(const blobdata_ref& tx_blob, transaction_prefix& tx); - bool parse_and_validate_tx_from_blob(const blobdata_ref& tx_blob, transaction& tx, crypto::hash& tx_hash, crypto::hash& tx_prefix_hash); - bool parse_and_validate_tx_from_blob(const blobdata_ref& tx_blob, transaction& tx, crypto::hash& tx_hash); - bool parse_and_validate_tx_from_blob(const blobdata_ref& tx_blob, transaction& tx); - bool parse_and_validate_tx_base_from_blob(const blobdata_ref& tx_blob, transaction& tx, const bool require_eof = false); + /* The size check is useful before parsing non-coinbase txs from untrusted sources. Coinbase blob sizes may be uncapped. */ + bool parse_and_validate_tx_prefix_from_blob(const blobdata_ref& tx_blob, transaction_prefix& tx, const bool max_size_check = false); + bool parse_and_validate_tx_from_blob(const blobdata_ref& tx_blob, transaction& tx, crypto::hash& tx_hash, crypto::hash& tx_prefix_hash, const bool max_size_check = false); + bool parse_and_validate_tx_from_blob(const blobdata_ref& tx_blob, transaction& tx, crypto::hash& tx_hash, const bool max_size_check = false); + bool parse_and_validate_tx_from_blob(const blobdata_ref& tx_blob, transaction& tx, const bool max_size_check = false); + bool parse_and_validate_tx_base_from_blob(const blobdata_ref& tx_blob, transaction& tx, const bool max_size_check = false, const bool require_eof = false); bool is_v1_tx(const blobdata_ref& tx_blob); bool is_v1_tx(const blobdata& tx_blob); diff --git a/src/cryptonote_core/blockchain.cpp b/src/cryptonote_core/blockchain.cpp index 983f79157..04c22c8ed 100644 --- a/src/cryptonote_core/blockchain.cpp +++ b/src/cryptonote_core/blockchain.cpp @@ -5371,7 +5371,7 @@ bool Blockchain::prepare_handle_incoming_blocks(const std::vector get_max_tx_size()) - { - MERROR("Transaction blob of length " << tx_entry.blob.size() << " is too large to unpack!"); - return false; - } - const bool is_pruned = tx_entry.prunable_hash != crypto::null_hash; if (is_pruned && !allow_pruned) { @@ -113,14 +107,15 @@ namespace cryptonote cryptonote::transaction tx; crypto::hash tx_hash; bool parse_success = false; + const bool max_size_check = true; if (is_pruned) { - if ((parse_success = cryptonote::parse_and_validate_tx_base_from_blob(tx_entry.blob, tx, /*require_eof=*/true))) + if ((parse_success = cryptonote::parse_and_validate_tx_base_from_blob(tx_entry.blob, tx, max_size_check, /*require_eof=*/true))) parse_success = cryptonote::get_pruned_transaction_hash(tx, tx_entry.prunable_hash, tx_hash); } else { - parse_success = cryptonote::parse_and_validate_tx_from_blob(tx_entry.blob, tx, tx_hash); + parse_success = cryptonote::parse_and_validate_tx_from_blob(tx_entry.blob, tx, tx_hash, max_size_check); } if (!parse_success) @@ -900,7 +895,7 @@ namespace cryptonote for (const auto &blob: arg.txs) { - MLOGIF_P2P_MESSAGE(cryptonote::transaction tx; crypto::hash hash; bool ret = cryptonote::parse_and_validate_tx_from_blob(blob, tx, hash);, ret, "Including transaction " << hash); + MLOGIF_P2P_MESSAGE(cryptonote::transaction tx; crypto::hash hash; bool ret = cryptonote::parse_and_validate_tx_from_blob(blob, tx, hash, true);, ret, "Including transaction " << hash); crypto::hash digest{}; if (!blob.empty()) diff --git a/src/wallet/wallet2.cpp b/src/wallet/wallet2.cpp index 17c2d5d77..b52891437 100644 --- a/src/wallet/wallet2.cpp +++ b/src/wallet/wallet2.cpp @@ -3177,7 +3177,7 @@ void wallet2::process_pool_info_extent(const cryptonote::COMMAND_RPC_GET_BLOCKS_ for (const auto &pool_tx: res.added_pool_txs) { cryptonote::transaction tx; - THROW_WALLET_EXCEPTION_IF(!cryptonote::parse_and_validate_tx_base_from_blob(pool_tx.tx_blob, tx), + THROW_WALLET_EXCEPTION_IF(!cryptonote::parse_and_validate_tx_base_from_blob(pool_tx.tx_blob, tx, true), error::wallet_internal_error, "Failed to validate transaction base from daemon"); added_pool_txs.push_back(std::make_tuple(tx, pool_tx.tx_hash, pool_tx.double_spend_seen)); }