From a6157fe0a9c36d214506738f3456dcfa7b86c436 Mon Sep 17 00:00:00 2001 From: Jason Rhinelander Date: Wed, 1 Jun 2022 15:44:24 -0300 Subject: [PATCH 1/3] Fail if tx blob is empty Some node on the network apparently has a corrupted DB and is spewing empty tx blobs onto the network. Detect such a case rather than broadcasting broken empty txes. --- src/blockchain_db/lmdb/db_lmdb.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/blockchain_db/lmdb/db_lmdb.cpp b/src/blockchain_db/lmdb/db_lmdb.cpp index 00c02b2080..f1f6efd23e 100644 --- a/src/blockchain_db/lmdb/db_lmdb.cpp +++ b/src/blockchain_db/lmdb/db_lmdb.cpp @@ -1998,6 +1998,9 @@ bool BlockchainLMDB::get_txpool_tx_blob(const crypto::hash& txid, std::string &b if (result != 0) throw1(DB_ERROR(lmdb_error("Error finding txpool tx blob: ", result).c_str())); + if (v.mv_size == 0) + throw1(DB_ERROR("Error finding txpool tx blob: tx is present, but data is empty")); + bd.assign(reinterpret_cast(v.mv_data), v.mv_size); return true; } From 4d91082acbcfa7cc494484b7b4184edb370179d9 Mon Sep 17 00:00:00 2001 From: Jason Rhinelander Date: Wed, 1 Jun 2022 15:46:01 -0300 Subject: [PATCH 2/3] Detect on_transaction_relayed failure If on_transaction_relayed fails to parse, it returns a null_hash, but that wasn't being detected here. Also eliminates calling `on_transaction_relayed(tx_blob)` twice. --- .../cryptonote_protocol_handler.inl | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/cryptonote_protocol/cryptonote_protocol_handler.inl b/src/cryptonote_protocol/cryptonote_protocol_handler.inl index a43dc0cd66..ad1b9ecd16 100644 --- a/src/cryptonote_protocol/cryptonote_protocol_handler.inl +++ b/src/cryptonote_protocol/cryptonote_protocol_handler.inl @@ -2579,17 +2579,15 @@ skip: template bool t_cryptonote_protocol_handler::relay_transactions(NOTIFY_NEW_TRANSACTIONS::request& arg, cryptonote_connection_context& exclude_context) { - for(auto& tx_blob : arg.txs) - m_core.on_transaction_relayed(tx_blob); - // no check for success, so tell core they're relayed unconditionally and snag a copy of the // hash so that we can look up any associated blink data we should include. std::vector relayed_txes; relayed_txes.reserve(arg.txs.size()); for (auto &tx_blob : arg.txs) - relayed_txes.push_back( - m_core.on_transaction_relayed(tx_blob) - ); + { + if (auto hash = m_core.on_transaction_relayed(tx_blob)) + relayed_txes.push_back(hash); + } // Rebuild arg.blinks from blink data that we have because we don't necessarily have the same // blink data that got sent to us (we may have additional blink info, or may have rejected some From b39a118daeea33919f8af0b4366ef365e7262a50 Mon Sep 17 00:00:00 2001 From: Jason Rhinelander Date: Wed, 1 Jun 2022 15:50:19 -0300 Subject: [PATCH 3/3] Detect and don't try to parse empty txes If we pass the blob into parse_and_validate_tx_from_blob we get `deserialization or varint failed` error logs, which are harmless bug annoying. This catches the empty tx blob case earlier and logs at a lesser severity. --- src/cryptonote_core/cryptonote_core.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/cryptonote_core/cryptonote_core.cpp b/src/cryptonote_core/cryptonote_core.cpp index c38943049a..a12ff5f852 100644 --- a/src/cryptonote_core/cryptonote_core.cpp +++ b/src/cryptonote_core/cryptonote_core.cpp @@ -1120,6 +1120,12 @@ namespace cryptonote tx_info.tvc.m_too_big = true; return; } + else if (tx_info.blob->empty()) + { + LOG_PRINT_L1("WRONG TRANSACTION BLOB, blob is empty, rejected"); + tx_info.tvc.m_verifivation_failed = true; + return; + } tx_info.parsed = parse_and_validate_tx_from_blob(*tx_info.blob, tx_info.tx, tx_info.tx_hash); if(!tx_info.parsed)