← Watch feed
cryptonote_basic: parse_and_validate_tx param to check max size
Public commit record
85/100 · Strong
What the developer wrote
Authored by j-berman
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.
✓ Specific, descriptive subject✓ Names a concrete action or component✓ Provides detailed explanatory context✓ Explains rationale or failure mode
What changed, and why it matters
This Monero commit adds an optional size check to transaction parsing functions so that oversized transaction blobs from untrusted network sources are rejected before being decoded. It is a defensive hardening change that consolidates existing size checks and extends them to more code paths, reducing the chance that a maliciously large blob could waste resources or trigger memory-related issues during parsing.
Recommended action
Treat as a security hardening improvement. Review that all network-facing callers now pass `max_size_check=true` and that `get_max_tx_size()` remains consistent with consensus rules. Consider whether any remaining untrusted callers still use the default false value. No immediate incident response is indicated by the commit alone.
Security signals we found
01
Adds explicit maximum-size validation before binary deserialization of untrusted transaction blobs
02
Enables size checks on network-facing transaction parsing paths (P2P protocol, core tx handling, relayed transactions)
03
Removes redundant size check in protocol handler in favor of centralized parsing API check
04
Comment notes coinbase transactions are intentionally exempt from the cap
05
Default parameter value of false preserves existing behavior for internal/trusted callers
Evidence from the diff
The patch introduces a max_size_check parameter (defaulting to false for backward compatibility) on parse_and_validate_tx_from_blob, parse_and_validate_tx_base_from_blob, and parse_and_validate_tx_prefix_from_blob. When enabled, it rejects blobs larger than get_max_tx_size() before binary deserialization. The commit then enables this check in the P2P protocol handler, core transaction handling, blockchain block preparation, tx sanity checks, and wallet pool processing. It also removes a redundant explicit size check in the protocol handler, moving it into the parsing API.
Changed components
src/cryptonote_basic/cryptonote_format_utils.cppsrc/cryptonote_basic/cryptonote_format_utils.hsrc/cryptonote_core/blockchain.cppsrc/cryptonote_core/cryptonote_core.cppsrc/cryptonote_core/tx_sanity_check.cppsrc/cryptonote_protocol/cryptonote_protocol_handler.inlsrc/wallet/wallet2.cppInspect captured patch +32 / −25
diff --git a/src/cryptonote_basic/cryptonote_format_utils.cpp b/src/cryptonote_basic/cryptonote_format_utils.cpp
index 93e4d74..759e166 100644
--- a/src/cryptonote_basic/cryptonote_format_utils.cpp
+++ b/src/cryptonote_basic/cryptonote_format_utils.cpp
@@ -120,6 +120,13 @@ namespace cryptonote
}
return total_key_offsets >= max_allowed;
}
+
+ 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();
+ }
//---------------------------------------------------------------
}
@@ -219,8 +226,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<false> ba{epee::strspan<std::uint8_t>(tx_blob)};
bool r = ::serialization::serialize(ba, tx);
CHECK_AND_ASSERT_MES(r, false, "Failed to parse transaction from blob");
@@ -231,8 +239,9 @@ namespace cryptonote
return true;
}
//---------------------------------------------------------------
- bool parse_and_validate_tx_base_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 max_size_check)
{
+ CHECK_AND_ASSERT_MES(passes_max_size_check(max_size_check, tx_blob), false, "Tx blob too big");
binary_archive<false> ba{epee::strspan<std::uint8_t>(tx_blob)};
bool r = tx.serialize_base(ba);
CHECK_AND_ASSERT_MES(r, false, "Failed to parse transaction from blob");
@@ -242,8 +251,9 @@ 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<false> ba{epee::strspan<std::uint8_t>(tx_blob)};
bool r = ::serialization::serialize_noeof(ba, tx);
CHECK_AND_ASSERT_MES(r, false, "Failed to parse transaction prefix from blob");
@@ -251,8 +261,9 @@ namespace cryptonote
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<false> ba{epee::strspan<std::uint8_t>(tx_blob)};
bool r = ::serialization::serialize(ba, tx);
CHECK_AND_ASSERT_MES(r, false, "Failed to parse transaction from blob");
@@ -265,9 +276,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 19e8cc2..8d019e3 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);
+ /* 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);
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 c2a119b..562fc8d 100644
--- a/src/cryptonote_core/blockchain.cpp
+++ b/src/cryptonote_core/blockchain.cpp
@@ -5060,7 +5060,7 @@ bool Blockchain::prepare_handle_incoming_blocks(const std::vector<block_complete
crypto::hash &tx_prefix_hash = txes[tx_index].second;
++tx_index;
- if (!parse_and_validate_tx_base_from_blob(tx_blob.blob, tx))
+ if (!parse_and_validate_tx_base_from_blob(tx_blob.blob, tx, true))
SCAN_TABLE_QUIT("Could not parse tx from incoming blocks.");
cryptonote::get_transaction_prefix_hash(tx, tx_prefix_hash);
diff --git a/src/cryptonote_core/cryptonote_core.cpp b/src/cryptonote_core/cryptonote_core.cpp
index 7eed808..4db58ea 100644
--- a/src/cryptonote_core/cryptonote_core.cpp
+++ b/src/cryptonote_core/cryptonote_core.cpp
@@ -784,7 +784,7 @@ namespace cryptonote
transaction tx;
crypto::hash txid;
- if (!parse_and_validate_tx_from_blob(tx_blob, tx, txid))
+ if (!parse_and_validate_tx_from_blob(tx_blob, tx, txid, true))
{
LOG_PRINT_L1("Incoming transactions failed to parse, rejected");
tvc.m_verifivation_failed = true;
@@ -1197,7 +1197,7 @@ namespace cryptonote
for (std::size_t i = 0; i < tx_blobs.size(); ++i)
{
- if (!parse_and_validate_tx_from_blob(tx_blobs[i], txs[i], tx_hashes[i]))
+ if (!parse_and_validate_tx_from_blob(tx_blobs[i], txs[i], tx_hashes[i], true))
{
LOG_ERROR("Failed to parse relayed transaction");
return;
diff --git a/src/cryptonote_core/tx_sanity_check.cpp b/src/cryptonote_core/tx_sanity_check.cpp
index 6925a2d..66e6521 100644
--- a/src/cryptonote_core/tx_sanity_check.cpp
+++ b/src/cryptonote_core/tx_sanity_check.cpp
@@ -43,7 +43,7 @@ bool tx_sanity_check(const cryptonote::blobdata &tx_blob, uint64_t rct_outs_avai
{
cryptonote::transaction tx;
- if (!cryptonote::parse_and_validate_tx_from_blob(tx_blob, tx))
+ if (!cryptonote::parse_and_validate_tx_from_blob(tx_blob, tx, true))
{
MERROR("Failed to parse transaction");
return false;
diff --git a/src/cryptonote_protocol/cryptonote_protocol_handler.inl b/src/cryptonote_protocol/cryptonote_protocol_handler.inl
index b9bb2bf..bb069f9 100644
--- a/src/cryptonote_protocol/cryptonote_protocol_handler.inl
+++ b/src/cryptonote_protocol/cryptonote_protocol_handler.inl
@@ -98,12 +98,6 @@ namespace cryptonote
for (const cryptonote::tx_blob_entry& tx_entry: tx_entries)
{
- if (tx_entry.blob.size() > 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)
{
@@ -114,14 +108,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)))
+ if ((parse_success = cryptonote::parse_and_validate_tx_base_from_blob(tx_entry.blob, tx, max_size_check)))
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)
@@ -931,7 +926,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 7194bc6..f5d7228 100644
--- a/src/wallet/wallet2.cpp
+++ b/src/wallet/wallet2.cpp
@@ -3152,7 +3152,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.emplace_back(std::move(tx), pool_tx.tx_hash, pool_tx.double_spend_seen);
}
Risk score
Our methodology →Why this scored 63/100
Human-validated context
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
No validated notes yet.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.