diff --git a/src/node/psbt.cpp b/src/node/psbt.cpp index 5c6a0b04a8..31aa293474 100644 --- a/src/node/psbt.cpp +++ b/src/node/psbt.cpp @@ -2,6 +2,7 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. +#include #include #include #include @@ -32,9 +33,17 @@ PSBTAnalysis AnalyzePSBT(PartiallySignedTransaction psbtx) CTxOut utxo; if (psbtx.GetInputUTXO(utxo, i)) { //TODO(gwillen) do PSBT inputs always have explicit assets & amounts? + if (!MoneyRange(utxo.nValue.GetAmount()) || !MoneyRange(in_amts[utxo.nAsset.GetAsset()] + utxo.nValue.GetAmount())) { + result.SetInvalid(strprintf("PSBT is not valid. Input %u has invalid value", i)); + return result; + } in_amts[utxo.nAsset.GetAsset()] += utxo.nValue.GetAmount(); input_analysis.has_utxo = true; } else { + if (input.non_witness_utxo && psbtx.tx->vin[i].prevout.n >= input.non_witness_utxo->vout.size()) { + result.SetInvalid(strprintf("PSBT is not valid. Input %u specifies invalid prevout", i)); + return result; + } input_analysis.has_utxo = false; input_analysis.is_final = false; input_analysis.next = PSBTRole::UPDATER; @@ -85,11 +94,23 @@ PSBTAnalysis AnalyzePSBT(PartiallySignedTransaction psbtx) if (calc_fee) { // Get the output amount CAmountMap out_amts = std::accumulate(psbtx.tx->vout.begin(), psbtx.tx->vout.end(), CAmountMap(), - [](CAmountMap map, const CTxOut& b) { - map[b.nAsset.GetAsset()] += b.nValue.GetAmount(); - return map; + [](CAmountMap a, const CTxOut& b) { + CAmount acc = a[b.nAsset.GetAsset()]; + CAmount add = b.nValue.GetAmount(); + + if (!MoneyRange(acc) || !MoneyRange(add) || !MoneyRange(acc + add)) { + CAmountMap invalid; + invalid[::policyAsset] = CAmount(-1); + return invalid; + } + a[b.nAsset.GetAsset()] += add; + return a; } ); + if (!MoneyRange(out_amts)) { + result.SetInvalid(strprintf("PSBT is not valid. Output amount invalid")); + return result; + } // Get the fee CAmountMap fee = in_amts - out_amts; diff --git a/src/psbt.cpp b/src/psbt.cpp index e7bf2a8dda..1388284d50 100644 --- a/src/psbt.cpp +++ b/src/psbt.cpp @@ -68,8 +68,11 @@ bool PartiallySignedTransaction::AddOutput(const CTxOut& txout, const PSBTOutput bool PartiallySignedTransaction::GetInputUTXO(CTxOut& utxo, int input_index) const { PSBTInput input = inputs[input_index]; - int prevout_index = tx->vin[input_index].prevout.n; + uint32_t prevout_index = tx->vin[input_index].prevout.n; if (input.non_witness_utxo) { + if (prevout_index >= input.non_witness_utxo->vout.size()) { + return false; + } utxo = input.non_witness_utxo->vout[prevout_index]; } else if (!input.witness_utxo.IsNull()) { utxo = input.witness_utxo; @@ -276,6 +279,9 @@ bool SignPSBTInput(const SigningProvider& provider, PartiallySignedTransaction& if (input.non_witness_utxo) { // If we're taking our information from a non-witness UTXO, verify that it matches the prevout. COutPoint prevout = tx.vin[index].prevout; + if (prevout.n >= input.non_witness_utxo->vout.size()) { + return false; + } if (input.non_witness_utxo->GetHash() != prevout.hash) { return false; } diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 0ef6d57263..8338e93a06 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -1269,7 +1269,8 @@ UniValue decodepsbt(const JSONRPCRequest& request) UniValue out(UniValue::VOBJ); if (txout.nValue.IsExplicit()) { - out.pushKV("amount", ValueFromAmount(txout.nValue.GetAmount())); + CAmount nValue = txout.nValue.GetAmount(); + out.pushKV("amount", ValueFromAmount(nValue)); } else { out.pushKV("amountcommitment", txout.nValue.GetHex()); } diff --git a/src/wallet/psbtwallet.cpp b/src/wallet/psbtwallet.cpp index e9008b16e6..31db0209e8 100644 --- a/src/wallet/psbtwallet.cpp +++ b/src/wallet/psbtwallet.cpp @@ -49,6 +49,9 @@ TransactionError FillPSBTInputsData(const CWallet* pwallet, PartiallySignedTrans if (!input.witness_utxo.IsNull()) { script = input.witness_utxo.scriptPubKey; } else if (input.non_witness_utxo) { + if (txin.prevout.n >= input.non_witness_utxo->vout.size()) { + return TransactionError::MISSING_INPUTS; + } script = input.non_witness_utxo->vout[txin.prevout.n].scriptPubKey; } else { // There's no UTXO so we can just skip this now @@ -177,6 +180,9 @@ TransactionError SignPSBT(const CWallet* pwallet, PartiallySignedTransaction& ps if (!input.witness_utxo.IsNull()) { script = input.witness_utxo.scriptPubKey; } else if (input.non_witness_utxo) { + if (txin.prevout.n >= input.non_witness_utxo->vout.size()) { + return TransactionError::MISSING_INPUTS; + } script = input.non_witness_utxo->vout[txin.prevout.n].scriptPubKey; } else { // There's no UTXO so we can just skip this now diff --git a/src/wallet/test/psbt_wallet_tests.cpp b/src/wallet/test/psbt_wallet_tests.cpp index 63f27e660a..c0c663b2d2 100644 --- a/src/wallet/test/psbt_wallet_tests.cpp +++ b/src/wallet/test/psbt_wallet_tests.cpp @@ -66,6 +66,15 @@ BOOST_AUTO_TEST_CASE(psbt_updater_test) ssTx << psbtx; std::string final_hex = HexStr(ssTx.begin(), ssTx.end()); BOOST_CHECK_EQUAL(final_hex, "70736274ff01009a020000000258e87a21b56daf0c23be8e7070456c336f7cbaa5c8757924f545887bb2abdd750000000000ffffffff838d0427d0ec650a68aa46bb0b098aea4422c071b2ca78352a077959d07cea1d0100000000ffffffff0270aaf00800000000160014d85c2b71d0060b09c9886aeb815e50991dda124d00e1f5050000000016001400aea9a2e5f0f876a588df5546e8742d1d87008f00000000000100bb0200000001aad73931018bd25f84ae400b68848be09db706eac2ac18298babee71ab656f8b0000000048473044022058f6fc7c6a33e1b31548d481c826c015bd30135aad42cd67790dab66d2ad243b02204a1ced2604c6735b6393e5b41691dd78b00f0c5942fb9f751856faa938157dba01feffffff0280f0fa020000000017a9140fb9463421696b82c833af241c78c17ddbde493487d0f20a270100000017a91429ca74f8a08f81999428185c97b5d852e4063f6187650000000104475221029583bf39ae0a609747ad199addd634fa6108559d6c5cd39b4c2183f1ab96e07f2102dab61ff49a14db6a7d02b0cd1fbb78fc4b18312b5b4e54dae4dba2fbfef536d752ae2206029583bf39ae0a609747ad199addd634fa6108559d6c5cd39b4c2183f1ab96e07f10d90c6a4f000000800000008000000080220602dab61ff49a14db6a7d02b0cd1fbb78fc4b18312b5b4e54dae4dba2fbfef536d710d90c6a4f00000080000000800100008001090880f0fa02000000000001012000c2eb0b0000000017a914b7f5faf40e3d40a5a459b1db3535f2b72fa921e88701042200208c2353173743b595dfb4a07b72ba8e42e3797da74e87fe7d9d7497e3b2028903010547522103089dc10c7ac6db54f91329af617333db388cead0c231f723379d1b99030b02dc21023add904f3d6dcf59ddb906b0dee23529b7ffb9ed50e5e86151926860221f0e7352ae2206023add904f3d6dcf59ddb906b0dee23529b7ffb9ed50e5e86151926860221f0e7310d90c6a4f000000800000008003000080220603089dc10c7ac6db54f91329af617333db388cead0c231f723379d1b99030b02dc10d90c6a4f00000080000000800200008001090800c2eb0b0000000000220203a9a4c37f5996d3aa25dbac6b570af0650394492942460b354753ed9eeca5877110d90c6a4f000000800000008004000080010521010000000000000000000000000000000000000000000000000000000000000000002202027f6399757d2eff55a136ad02c684b1838b6556e5f1b6b34282a94b6b5005109610d90c6a4f00000080000000800500008001052101000000000000000000000000000000000000000000000000000000000000000000"); + + // Mutate the transaction so that one of the inputs is invalid + psbtx.tx->vin[0].prevout.n = 2; + + // Try to sign the mutated input + SignatureData sigdata; + psbtx.inputs[0].FillSignatureData(sigdata); + const SigningProvider* provider = m_wallet.GetSigningProvider(ws1, sigdata); + BOOST_CHECK(!SignPSBTInput(*provider, psbtx, 0, SIGHASH_ALL)); } BOOST_AUTO_TEST_CASE(parse_hd_keypath) diff --git a/test/functional/rpc_psbt.py b/test/functional/rpc_psbt.py index 96416f6857..e865ca77d5 100755 --- a/test/functional/rpc_psbt.py +++ b/test/functional/rpc_psbt.py @@ -549,6 +549,8 @@ class PSBTTest(BitcoinTestFramework): self.nodes[0].generate(200) self.sync_all() + assert_raises_rpc_error(-25, 'Missing inputs', self.nodes[0].walletprocesspsbt, 'cHNldP8BAOECAAAAAAK2GUhaoOw0nV0BJhZg0a8G7vb9PQvlTds8GA/N+l+peAMAAAAA/////7YZSFqg7DSdXQEmFmDRrwbu9v09C+VN2zwYD836X6l4AQAAAAD/////AgEAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAEAAAAAlQK2wAAWABSNJKzjaUb3uOxixsvh1GGE3fW7zQEAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAEAAAAAlQL5AAAWABQo3DTHwdFy0CCa+h6+bi7VJs3tcgAAAAAAAQDhAgAAAAACczMa321tVHuN4GKWKRncycI22aX3uXgwSFUKM2orjRsBAAAAAP////9zMxrfbW1Ue43gYpYpGdzJwjbZpfe5eDBIVQozaiuNGwAAAAAA/////wIBAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAABAAAAAJUC+QAAFgAU/c11FQPC7OYlaspyx3uf70ntV8kBAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAABAAAAAJUCzUAAFgAUZlkFHI0o4IUPP7h+AZ/85GpMC7QAAAAAAAEBQgEAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAEAAAAAlQLNQAAWABRmWQUcjSjghQ8/uH4Bn/zkakwLtAAAAA==') + # Run all the pre-Elements, tests first with non-confidential addresses, then again with confidential addresses self.run_basic_tests(False) self.run_basic_tests(True) @@ -668,5 +670,20 @@ class PSBTTest(BitcoinTestFramework): assert_equal(analysis['next'], 'creator') assert_equal(analysis['error'], 'PSBT is not valid. Input 0 spends unspendable output') + self.log.info("PSBT with invalid values should have error message and Creator as next") + analysis = self.nodes[0].analyzepsbt('cHNldP8BALgCAAAAAAHwNNARYAJurafOkaMMB+gTCJkDS+c11HE0/e16Cxs9AQAAAAAA/////wIBAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAABAAAAAJUC+QAAFgAUKNw0x8HRctAgmvoevm4u1SbN7XIBAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAABAAAAAJUC7fwAFgAU9yTiAXuIvg0vjC19EAqBBuCGJNQAAAAAAAEBQgEAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAEAB9DjaoGAAAAWABSVA7cX9jx6OuNRxDgTgCLxTDU69gAAAA==') + assert_equal(analysis['next'], 'creator') + assert_equal(analysis['error'], 'PSBT is not valid. Input 0 has invalid value') + + analysis = self.nodes[0].analyzepsbt('cHNldP8BALgCAAAAAAHwNNARYAJurafOkaMMB+gTCJkDS+c11HE0/e16Cxs9AQAAAAAA/////wIBAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAABAAfQ42qBgAAAFgAUKNw0x8HRctAgmvoevm4u1SbN7XIBAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAABAAAAAJUC7fwAFgAU9yTiAXuIvg0vjC19EAqBBuCGJNQAAAAAAAEBQgEAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAEAAAABKgXyAAAWABSVA7cX9jx6OuNRxDgTgCLxTDU69gAAAA==') + assert_equal(analysis['next'], 'creator') + assert_equal(analysis['error'], 'PSBT is not valid. Output amount invalid') + + analysis = self.nodes[0].analyzepsbt('cHNldP8BAOECAAAAAAK2GUhaoOw0nV0BJhZg0a8G7vb9PQvlTds8GA/N+l+peAMAAAAA/////7YZSFqg7DSdXQEmFmDRrwbu9v09C+VN2zwYD836X6l4AQAAAAD/////AgEAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAEAAAAAlQK2wAAWABSNJKzjaUb3uOxixsvh1GGE3fW7zQEAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAEAAAAAlQL5AAAWABQo3DTHwdFy0CCa+h6+bi7VJs3tcgAAAAAAAQDhAgAAAAACczMa321tVHuN4GKWKRncycI22aX3uXgwSFUKM2orjRsBAAAAAP////9zMxrfbW1Ue43gYpYpGdzJwjbZpfe5eDBIVQozaiuNGwAAAAAA/////wIBAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAABAAAAAJUC+QAAFgAU/c11FQPC7OYlaspyx3uf70ntV8kBAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAABAAAAAJUCzUAAFgAUZlkFHI0o4IUPP7h+AZ/85GpMC7QAAAAAAAEBQgEAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAEAAAAAlQLNQAAWABRmWQUcjSjghQ8/uH4Bn/zkakwLtAAAAA==') + assert_equal(analysis['next'], 'creator') + assert_equal(analysis['error'], 'PSBT is not valid. Input 0 specifies invalid prevout') + + assert_raises_rpc_error(-25, 'Missing inputs', self.nodes[0].walletprocesspsbt, 'cHNldP8BAOECAAAAAAK2GUhaoOw0nV0BJhZg0a8G7vb9PQvlTds8GA/N+l+peAMAAAAA/////7YZSFqg7DSdXQEmFmDRrwbu9v09C+VN2zwYD836X6l4AQAAAAD/////AgEAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAEAAAAAlQK2wAAWABSNJKzjaUb3uOxixsvh1GGE3fW7zQEAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAEAAAAAlQL5AAAWABQo3DTHwdFy0CCa+h6+bi7VJs3tcgAAAAAAAQDhAgAAAAACczMa321tVHuN4GKWKRncycI22aX3uXgwSFUKM2orjRsBAAAAAP////9zMxrfbW1Ue43gYpYpGdzJwjbZpfe5eDBIVQozaiuNGwAAAAAA/////wIBAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAABAAAAAJUC+QAAFgAU/c11FQPC7OYlaspyx3uf70ntV8kBAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAABAAAAAJUCzUAAFgAUZlkFHI0o4IUPP7h+AZ/85GpMC7QAAAAAAAEBQgEAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAEAAAAAlQLNQAAWABRmWQUcjSjghQ8/uH4Bn/zkakwLtAAAAA==') + if __name__ == '__main__': PSBTTest().main()