Merge #17156: psbt: check that various indexes and amounts are within bounds

deaa6dd144 psbt: check output index is within bounds before accessing (Andrew Chow)
f1ef7f0aa4 Don't calculate tx fees for PSBTs with invalid money values (Andrew Chow)

Pull request description:

  Fixes #17149

  Two classes of issues were found by the psbt fuzzer: values out of range and causing overflows, and prevout indexes being out of range. This PR fixes both.

  When accessing a specific output using the index given in the tx, check that it is actually a possible output before trying to access the output.

  When summing and checking amounts for `decodepsbt` and `analyzepsbt`, make sure that the values are actually valid money values.. Otherwise, stop summing and don't show the fee. For `analyzepsbt`, return that the next role is the Creator since the Creator needs to remake the transaction to be valid.

ACKs for top commit:
  practicalswift:
    ACK deaa6dd144 -- only change since last ACK was the addition of tests
  gwillen:
    tested ACK deaa6dd, would also like to see this merged!

Tree-SHA512: 06c36720bbb5a7ab1c29f7d15878bf9f0d3e5760c06bff479d412e1bf07bb3e0e9ab6cca820a4bfedaab71bfd7af813807e87cbcdf0af25cc3f66a53a06dbcfd
This commit is contained in:
fanquake
2020-01-29 19:26:17 +08:00
6 changed files with 69 additions and 4 deletions

View File

@@ -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 <amount.h>
#include <coins.h>
#include <consensus/tx_verify.h>
#include <node/psbt.h>
@@ -31,9 +32,17 @@ PSBTAnalysis AnalyzePSBT(PartiallySignedTransaction psbtx)
// Check for a UTXO
CTxOut utxo;
if (psbtx.GetInputUTXO(utxo, i)) {
if (!MoneyRange(utxo.nValue) || !MoneyRange(in_amt + utxo.nValue)) {
result.SetInvalid(strprintf("PSBT is not valid. Input %u has invalid value", i));
return result;
}
in_amt += utxo.nValue;
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,9 +94,16 @@ PSBTAnalysis AnalyzePSBT(PartiallySignedTransaction psbtx)
// Get the output amount
CAmount out_amt = std::accumulate(psbtx.tx->vout.begin(), psbtx.tx->vout.end(), CAmount(0),
[](CAmount a, const CTxOut& b) {
if (!MoneyRange(a) || !MoneyRange(b.nValue) || !MoneyRange(a + b.nValue)) {
return CAmount(-1);
}
return a += b.nValue;
}
);
if (!MoneyRange(out_amt)) {
result.SetInvalid(strprintf("PSBT is not valid. Output amount invalid"));
return result;
}
// Get the fee
CAmount fee = in_amt - out_amt;