cryptonote_basic: pruned hash return bool
What the developer wrote
Authored by j-berman
cryptonote_basic: pruned hash return bool
What changed, and why it matters
This small code change converts a function that computes a pruned transaction hash from one that throws exceptions on failure to one that returns a success/failure boolean. Callers now check that boolean and stop processing if the hash could not be calculated. The main practical effect is to prevent malformed or version-1 pruned transactions from causing unhandled exceptions or being processed with an unset hash, which could lead to denial-of-service or incorrect wallet/node behavior.
Treat as a low-severity hardening fix. Review whether any other callers of get_pruned_transaction_hash exist in supported branches and ensure they also check the return value. Consider whether the previous exception-throwing behavior could have been reachable from network input and whether backporting is warranted for stable releases.
Security signals we found
Error-handling conversion from exception-throwing to boolean-returning
Addition of caller-side validation for pruned transaction hash computation
Prevents unhandled exceptions on v1 pruned transactions or serialization failures
Hardens transaction parsing in P2P protocol and wallet code paths
Evidence from the diff
The commit changes get_pruned_transaction_hash() from returning crypto::hash and throwing on error (CHECK_AND_ASSERT_THROW_MES) to returning bool and writing the hash into an out-parameter (CHECK_AND_ASSERT_MES with false). Callers in cryptonote_protocol_handler.inl and wallet2.cpp now propagate or check the boolean. This is a hardening change: previously, a v1 pruned transaction or a failure to serialize RCT signatures would raise an exception; now the failure is returned and callers can handle it gracefully. The change reduces the attack surface for malformed transaction blobs causing crashes or inconsistent state.
Changed components
src/cryptonote_basic/cryptonote_format_utils.cppsrc/cryptonote_basic/cryptonote_format_utils.hsrc/cryptonote_protocol/cryptonote_protocol_handler.inlsrc/wallet/wallet2.cppInspect captured patch +8 / −8
diff --git a/src/cryptonote_basic/cryptonote_format_utils.cpp b/src/cryptonote_basic/cryptonote_format_utils.cpp
index 7e4a35c..e4ef2b1 100644
--- a/src/cryptonote_basic/cryptonote_format_utils.cpp
+++ b/src/cryptonote_basic/cryptonote_format_utils.cpp
@@ -1313,10 +1313,10 @@ namespace cryptonote
return res;
}
//---------------------------------------------------------------
- crypto::hash get_pruned_transaction_hash(const transaction& t, const crypto::hash &pruned_data_hash)
+ bool get_pruned_transaction_hash(const transaction& t, const crypto::hash &pruned_data_hash, crypto::hash &res)
{
// v1 transactions hash the entire blob
- CHECK_AND_ASSERT_THROW_MES(t.version > 1, "Hash for pruned v1 tx cannot be calculated");
+ CHECK_AND_ASSERT_MES(t.version > 1, false, "Hash for pruned v1 tx cannot be calculated");
// v2 transactions hash different parts together, than hash the set of those hashes
crypto::hash hashes[3];
@@ -1333,7 +1333,7 @@ namespace cryptonote
const size_t inputs = t.vin.size();
const size_t outputs = t.vout.size();
bool r = tt.rct_signatures.serialize_rctsig_base(ba, inputs, outputs);
- CHECK_AND_ASSERT_THROW_MES(r, "Failed to serialize rct signatures base");
+ CHECK_AND_ASSERT_MES(r, false, "Failed to serialize rct signatures base");
cryptonote::get_blob_hash(ss.str(), hashes[1]);
}
@@ -1344,9 +1344,9 @@ namespace cryptonote
hashes[2] = pruned_data_hash;
// the tx hash is the hash of the 3 hashes
- crypto::hash res = cn_fast_hash(hashes, sizeof(hashes));
+ res = cn_fast_hash(hashes, sizeof(hashes));
t.set_hash(res);
- return res;
+ return true;
}
//---------------------------------------------------------------
bool calculate_transaction_hash(const transaction& t, crypto::hash& res, size_t* blob_size)
diff --git a/src/cryptonote_basic/cryptonote_format_utils.h b/src/cryptonote_basic/cryptonote_format_utils.h
index 64383ca..19e8cc2 100644
--- a/src/cryptonote_basic/cryptonote_format_utils.h
+++ b/src/cryptonote_basic/cryptonote_format_utils.h
@@ -118,7 +118,7 @@ namespace cryptonote
bool calculate_transaction_prunable_hash(const transaction& t, const cryptonote::blobdata_ref *blob, crypto::hash& res);
crypto::hash get_transaction_prunable_hash(const transaction& t, const cryptonote::blobdata_ref *blob = NULL);
bool calculate_transaction_hash(const transaction& t, crypto::hash& res, size_t* blob_size);
- crypto::hash get_pruned_transaction_hash(const transaction& t, const crypto::hash &pruned_data_hash);
+ bool get_pruned_transaction_hash(const transaction& t, const crypto::hash &pruned_data_hash, crypto::hash& res);
blobdata get_block_hashing_blob(const block& b);
bool calculate_block_hash(const block& b, crypto::hash& res, const blobdata_ref *blob = NULL);
diff --git a/src/cryptonote_protocol/cryptonote_protocol_handler.inl b/src/cryptonote_protocol/cryptonote_protocol_handler.inl
index bcf4471..662618d 100644
--- a/src/cryptonote_protocol/cryptonote_protocol_handler.inl
+++ b/src/cryptonote_protocol/cryptonote_protocol_handler.inl
@@ -119,7 +119,7 @@ namespace cryptonote
if (is_pruned)
{
if ((parse_success = cryptonote::parse_and_validate_tx_base_from_blob(tx_entry.blob, tx)))
- tx_hash = cryptonote::get_pruned_transaction_hash(tx, tx_entry.prunable_hash);
+ parse_success = cryptonote::get_pruned_transaction_hash(tx, tx_entry.prunable_hash, tx_hash);
}
else
{
diff --git a/src/wallet/wallet2.cpp b/src/wallet/wallet2.cpp
index 1f70f22..ba757d9 100644
--- a/src/wallet/wallet2.cpp
+++ b/src/wallet/wallet2.cpp
@@ -984,7 +984,7 @@ bool get_pruned_tx(const cryptonote::COMMAND_RPC_GET_TRANSACTIONS::entry &entry,
// only v2 txes can calculate their txid after pruned
if (bd[0] > 1)
{
- tx_hash = cryptonote::get_pruned_transaction_hash(tx, ph);
+ CHECK_AND_ASSERT_MES(cryptonote::get_pruned_transaction_hash(tx, ph, tx_hash), false, "Failed to get pruned tx hash");
}
else
{
Why this scored 34/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.