From 53a75ebbaec5e982713f04629fbbb04f0e28d137 Mon Sep 17 00:00:00 2001 From: sanket1729 Date: Mon, 8 Aug 2022 03:28:51 -0700 Subject: [PATCH] Check the value assetion only on valid amounts This causes crash on elements wallet when dealing with transactions that have explicit values and confidential assets. This creates a somewhat serious DoS attack as the sender can cause the reciever's wallet to crash by partially blinding the change output. To make matters worse, the wallet initially accepts the transaction, but fails while spending the output. This is likely caused by a combination of two bugs: 1) The wallet's current behaviour stores the complete transaction of interest in CWalletTx instead of just Outpoints. Only that the spend time do we iterate over all outputs, try to unblind them and check which are isMine. When calling wtx.GetOutputValueOut() or similar calls, we hit this assertion. While the current behaviour is okay, I think the correct way is move the IsMine == ISMINE_NO at the start of the loop. We should not do be any checks on outputs that are not ours. This is used in multiple places at different parts of the codebase for different RPCs. 2) When dealing with partially blinded trasactions, ComputeBlindingData correctly sets value = -1, and the cache byte to 1. When getting the data again with GetBlindingData for explicit value and confidential asset, we load the precomputed data with value = -1 and assert the loaded value be the explicit value in the transaction. This is only true for explicit value and explicit asset. The changed assertion checks that written value should be same as the explicit value that was written only when the amounts are valid --- src/wallet/wallet.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 0615a69417..6366e5e213 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -3464,7 +3464,7 @@ void CWalletTx::GetBlindingData(const unsigned int map_index, const std::vector< memcpy(asset_tag.begin(), &*(it + 73), 32); pubkey.Set(it + 105, it + 138); - if (conf_value.IsExplicit()) { + if (amount != -1 && conf_value.IsExplicit()) { assert(conf_value.GetAmount() == amount); } } else {