From b77b0cc507bdc716e5236b1d9880e648147e0af9 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Mon, 1 Mar 2021 16:49:59 -0800 Subject: Add MissingDataBehavior and make TransactionSignatureChecker handle it This allows specifying how *TransactionSignatureChecker will behave when presented with missing transaction data such as amounts spent, BIP341 data, or spent outputs. As all call sites still (implicitly) use MissingDataBehavior::ASSERT_FAIL, this commit introduces no change in behavior. --- src/script/interpreter.cpp | 20 +++++++++++++++++--- src/script/interpreter.h | 14 ++++++++++++-- 2 files changed, 29 insertions(+), 5 deletions(-) (limited to 'src') diff --git a/src/script/interpreter.cpp b/src/script/interpreter.cpp index 20a4ce48b0..2b10f5b0df 100644 --- a/src/script/interpreter.cpp +++ b/src/script/interpreter.cpp @@ -1488,8 +1488,20 @@ static const CHashWriter HASHER_TAPLEAF = TaggedHash("TapLeaf"); static const CHashWriter HASHER_TAPBRANCH = TaggedHash("TapBranch"); static const CHashWriter HASHER_TAPTWEAK = TaggedHash("TapTweak"); +static bool HandleMissingData(MissingDataBehavior mdb) +{ + switch (mdb) { + case MissingDataBehavior::ASSERT_FAIL: + assert(!"Missing data"); + break; + case MissingDataBehavior::FAIL: + return false; + } + assert(!"Unknown MissingDataBehavior value"); +} + template -bool SignatureHashSchnorr(uint256& hash_out, const ScriptExecutionData& execdata, const T& tx_to, uint32_t in_pos, uint8_t hash_type, SigVersion sigversion, const PrecomputedTransactionData& cache) +bool SignatureHashSchnorr(uint256& hash_out, const ScriptExecutionData& execdata, const T& tx_to, uint32_t in_pos, uint8_t hash_type, SigVersion sigversion, const PrecomputedTransactionData& cache, MissingDataBehavior mdb) { uint8_t ext_flag, key_version; switch (sigversion) { @@ -1509,7 +1521,9 @@ bool SignatureHashSchnorr(uint256& hash_out, const ScriptExecutionData& execdata assert(false); } assert(in_pos < tx_to.vin.size()); - assert(cache.m_bip341_taproot_ready && cache.m_spent_outputs_ready); + if (!(cache.m_bip341_taproot_ready && cache.m_spent_outputs_ready)) { + return HandleMissingData(mdb); + } CHashWriter ss = HASHER_TAPSIGHASH; @@ -1696,7 +1710,7 @@ bool GenericTransactionSignatureChecker::CheckSchnorrSignature(Spantxdata); - if (!SignatureHashSchnorr(sighash, execdata, *txTo, nIn, hashtype, sigversion, *this->txdata)) { + if (!SignatureHashSchnorr(sighash, execdata, *txTo, nIn, hashtype, sigversion, *this->txdata, m_mdb)) { return set_error(serror, SCRIPT_ERR_SCHNORR_SIG_HASHTYPE); } if (!VerifySchnorrSignature(sig, pubkey, sighash)) return set_error(serror, SCRIPT_ERR_SCHNORR_SIG); diff --git a/src/script/interpreter.h b/src/script/interpreter.h index b4c163c841..490b337108 100644 --- a/src/script/interpreter.h +++ b/src/script/interpreter.h @@ -247,11 +247,21 @@ public: virtual ~BaseSignatureChecker() {} }; +/** Enum to specify what *TransactionSignatureChecker's behavior should be + * when dealing with missing transaction data. + */ +enum class MissingDataBehavior +{ + ASSERT_FAIL, //!< Abort execution through assertion failure (for consensus code) + FAIL, //!< Just act as if the signature was invalid +}; + template class GenericTransactionSignatureChecker : public BaseSignatureChecker { private: const T* txTo; + const MissingDataBehavior m_mdb; unsigned int nIn; const CAmount amount; const PrecomputedTransactionData* txdata; @@ -261,8 +271,8 @@ protected: virtual bool VerifySchnorrSignature(Span sig, const XOnlyPubKey& pubkey, const uint256& sighash) const; public: - GenericTransactionSignatureChecker(const T* txToIn, unsigned int nInIn, const CAmount& amountIn) : txTo(txToIn), nIn(nInIn), amount(amountIn), txdata(nullptr) {} - GenericTransactionSignatureChecker(const T* txToIn, unsigned int nInIn, const CAmount& amountIn, const PrecomputedTransactionData& txdataIn) : txTo(txToIn), nIn(nInIn), amount(amountIn), txdata(&txdataIn) {} + GenericTransactionSignatureChecker(const T* txToIn, unsigned int nInIn, const CAmount& amountIn, MissingDataBehavior mdb = MissingDataBehavior::ASSERT_FAIL) : txTo(txToIn), m_mdb(mdb), nIn(nInIn), amount(amountIn), txdata(nullptr) {} + GenericTransactionSignatureChecker(const T* txToIn, unsigned int nInIn, const CAmount& amountIn, const PrecomputedTransactionData& txdataIn, MissingDataBehavior mdb = MissingDataBehavior::ASSERT_FAIL) : txTo(txToIn), m_mdb(mdb), nIn(nInIn), amount(amountIn), txdata(&txdataIn) {} bool CheckECDSASignature(const std::vector& scriptSig, const std::vector& vchPubKey, const CScript& scriptCode, SigVersion sigversion) const override; bool CheckSchnorrSignature(Span sig, Span pubkey, SigVersion sigversion, const ScriptExecutionData& execdata, ScriptError* serror = nullptr) const override; bool CheckLockTime(const CScriptNum& nLockTime) const override; -- cgit v1.2.3 From 3820090bd619ac85ab35eff376c03136fe4a9f04 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Mon, 1 Mar 2021 18:07:14 -0800 Subject: Make all SignatureChecker explicit about missing data Remove the implicit MissingDataBehavior::ASSERT_FAIL in the *TransationSignatureChecker constructors, and instead specify it explicit in all call sites: * Test code uses ASSERT_FAIL * Validation uses ASSERT_FAIL (through CachingTransactionSignatureChecker) (including signet) * libconsensus uses FAIL, matching the existing behavior of the non-amount API (and the extended required data for taproot validation is not available yet) * Signing code uses FAIL --- src/bench/verify_script.cpp | 2 +- src/script/bitcoinconsensus.cpp | 2 +- src/script/interpreter.h | 4 ++-- src/script/sigcache.h | 2 +- src/script/sign.cpp | 6 +++--- src/signet.cpp | 2 +- src/test/fuzz/script_assets_test_minimizer.cpp | 4 ++-- src/test/fuzz/script_flags.cpp | 2 +- src/test/multisig_tests.cpp | 16 +++++++------- src/test/script_p2sh_tests.cpp | 2 +- src/test/script_tests.cpp | 30 +++++++++++++------------- src/test/sigopcount_tests.cpp | 2 +- src/test/transaction_tests.cpp | 4 ++-- 13 files changed, 39 insertions(+), 39 deletions(-) (limited to 'src') diff --git a/src/bench/verify_script.cpp b/src/bench/verify_script.cpp index e3f6b35a7d..39e74b9b2b 100644 --- a/src/bench/verify_script.cpp +++ b/src/bench/verify_script.cpp @@ -56,7 +56,7 @@ static void VerifyScriptBench(benchmark::Bench& bench) txCredit.vout[0].scriptPubKey, &txSpend.vin[0].scriptWitness, flags, - MutableTransactionSignatureChecker(&txSpend, 0, txCredit.vout[0].nValue), + MutableTransactionSignatureChecker(&txSpend, 0, txCredit.vout[0].nValue, MissingDataBehavior::ASSERT_FAIL), &err); assert(err == SCRIPT_ERR_OK); assert(success); diff --git a/src/script/bitcoinconsensus.cpp b/src/script/bitcoinconsensus.cpp index 76609f01a7..a9aa6a0060 100644 --- a/src/script/bitcoinconsensus.cpp +++ b/src/script/bitcoinconsensus.cpp @@ -92,7 +92,7 @@ static int verify_script(const unsigned char *scriptPubKey, unsigned int scriptP set_error(err, bitcoinconsensus_ERR_OK); PrecomputedTransactionData txdata(tx); - return VerifyScript(tx.vin[nIn].scriptSig, CScript(scriptPubKey, scriptPubKey + scriptPubKeyLen), &tx.vin[nIn].scriptWitness, flags, TransactionSignatureChecker(&tx, nIn, amount, txdata), nullptr); + return VerifyScript(tx.vin[nIn].scriptSig, CScript(scriptPubKey, scriptPubKey + scriptPubKeyLen), &tx.vin[nIn].scriptWitness, flags, TransactionSignatureChecker(&tx, nIn, amount, txdata, MissingDataBehavior::FAIL), nullptr); } catch (const std::exception&) { return set_error(err, bitcoinconsensus_ERR_TX_DESERIALIZE); // Error deserializing } diff --git a/src/script/interpreter.h b/src/script/interpreter.h index 490b337108..970543ce38 100644 --- a/src/script/interpreter.h +++ b/src/script/interpreter.h @@ -271,8 +271,8 @@ protected: virtual bool VerifySchnorrSignature(Span sig, const XOnlyPubKey& pubkey, const uint256& sighash) const; public: - GenericTransactionSignatureChecker(const T* txToIn, unsigned int nInIn, const CAmount& amountIn, MissingDataBehavior mdb = MissingDataBehavior::ASSERT_FAIL) : txTo(txToIn), m_mdb(mdb), nIn(nInIn), amount(amountIn), txdata(nullptr) {} - GenericTransactionSignatureChecker(const T* txToIn, unsigned int nInIn, const CAmount& amountIn, const PrecomputedTransactionData& txdataIn, MissingDataBehavior mdb = MissingDataBehavior::ASSERT_FAIL) : txTo(txToIn), m_mdb(mdb), nIn(nInIn), amount(amountIn), txdata(&txdataIn) {} + GenericTransactionSignatureChecker(const T* txToIn, unsigned int nInIn, const CAmount& amountIn, MissingDataBehavior mdb) : txTo(txToIn), m_mdb(mdb), nIn(nInIn), amount(amountIn), txdata(nullptr) {} + GenericTransactionSignatureChecker(const T* txToIn, unsigned int nInIn, const CAmount& amountIn, const PrecomputedTransactionData& txdataIn, MissingDataBehavior mdb) : txTo(txToIn), m_mdb(mdb), nIn(nInIn), amount(amountIn), txdata(&txdataIn) {} bool CheckECDSASignature(const std::vector& scriptSig, const std::vector& vchPubKey, const CScript& scriptCode, SigVersion sigversion) const override; bool CheckSchnorrSignature(Span sig, Span pubkey, SigVersion sigversion, const ScriptExecutionData& execdata, ScriptError* serror = nullptr) const override; bool CheckLockTime(const CScriptNum& nLockTime) const override; diff --git a/src/script/sigcache.h b/src/script/sigcache.h index bf0ba38c2d..7b6b91c963 100644 --- a/src/script/sigcache.h +++ b/src/script/sigcache.h @@ -27,7 +27,7 @@ private: bool store; public: - CachingTransactionSignatureChecker(const CTransaction* txToIn, unsigned int nInIn, const CAmount& amountIn, bool storeIn, PrecomputedTransactionData& txdataIn) : TransactionSignatureChecker(txToIn, nInIn, amountIn, txdataIn), store(storeIn) {} + CachingTransactionSignatureChecker(const CTransaction* txToIn, unsigned int nInIn, const CAmount& amountIn, bool storeIn, PrecomputedTransactionData& txdataIn) : TransactionSignatureChecker(txToIn, nInIn, amountIn, txdataIn, MissingDataBehavior::ASSERT_FAIL), store(storeIn) {} bool VerifyECDSASignature(const std::vector& vchSig, const CPubKey& vchPubKey, const uint256& sighash) const override; bool VerifySchnorrSignature(Span sig, const XOnlyPubKey& pubkey, const uint256& sighash) const override; diff --git a/src/script/sign.cpp b/src/script/sign.cpp index dba5ce621a..fb3466f320 100644 --- a/src/script/sign.cpp +++ b/src/script/sign.cpp @@ -14,7 +14,7 @@ typedef std::vector valtype; -MutableTransactionSignatureCreator::MutableTransactionSignatureCreator(const CMutableTransaction* txToIn, unsigned int nInIn, const CAmount& amountIn, int nHashTypeIn) : txTo(txToIn), nIn(nInIn), nHashType(nHashTypeIn), amount(amountIn), checker(txTo, nIn, amountIn) {} +MutableTransactionSignatureCreator::MutableTransactionSignatureCreator(const CMutableTransaction* txToIn, unsigned int nInIn, const CAmount& amountIn, int nHashTypeIn) : txTo(txToIn), nIn(nInIn), nHashType(nHashTypeIn), amount(amountIn), checker(txTo, nIn, amountIn, MissingDataBehavior::FAIL) {} bool MutableTransactionSignatureCreator::CreateSig(const SigningProvider& provider, std::vector& vchSig, const CKeyID& address, const CScript& scriptCode, SigVersion sigversion) const { @@ -292,7 +292,7 @@ SignatureData DataFromTransaction(const CMutableTransaction& tx, unsigned int nI Stacks stack(data); // Get signatures - MutableTransactionSignatureChecker tx_checker(&tx, nIn, txout.nValue); + MutableTransactionSignatureChecker tx_checker(&tx, nIn, txout.nValue, MissingDataBehavior::FAIL); SignatureExtractorChecker extractor_checker(data, tx_checker); if (VerifyScript(data.scriptSig, txout.scriptPubKey, &data.scriptWitness, STANDARD_SCRIPT_VERIFY_FLAGS, extractor_checker)) { data.complete = true; @@ -499,7 +499,7 @@ bool SignTransaction(CMutableTransaction& mtx, const SigningProvider* keystore, } ScriptError serror = SCRIPT_ERR_OK; - if (!VerifyScript(txin.scriptSig, prevPubKey, &txin.scriptWitness, STANDARD_SCRIPT_VERIFY_FLAGS, TransactionSignatureChecker(&txConst, i, amount), &serror)) { + if (!VerifyScript(txin.scriptSig, prevPubKey, &txin.scriptWitness, STANDARD_SCRIPT_VERIFY_FLAGS, TransactionSignatureChecker(&txConst, i, amount, MissingDataBehavior::FAIL), &serror)) { if (serror == SCRIPT_ERR_INVALID_STACK_OPERATION) { // Unable to sign input and verification failed (possible attempt to partially sign). input_errors[i] = "Unable to sign input, invalid stack size (possibly missing key)"; diff --git a/src/signet.cpp b/src/signet.cpp index e68f031aa4..e41b94da8d 100644 --- a/src/signet.cpp +++ b/src/signet.cpp @@ -139,7 +139,7 @@ bool CheckSignetBlockSolution(const CBlock& block, const Consensus::Params& cons const CScript& scriptSig = signet_txs->m_to_sign.vin[0].scriptSig; const CScriptWitness& witness = signet_txs->m_to_sign.vin[0].scriptWitness; - TransactionSignatureChecker sigcheck(&signet_txs->m_to_sign, /*nIn=*/ 0, /*amount=*/ signet_txs->m_to_spend.vout[0].nValue); + TransactionSignatureChecker sigcheck(&signet_txs->m_to_sign, /*nIn=*/ 0, /*amount=*/ signet_txs->m_to_spend.vout[0].nValue, MissingDataBehavior::ASSERT_FAIL); if (!VerifyScript(scriptSig, signet_txs->m_to_spend.vout[0].scriptPubKey, &witness, BLOCK_SCRIPT_VERIFY_FLAGS, sigcheck)) { LogPrint(BCLog::VALIDATION, "CheckSignetBlockSolution: Errors in block (block solution invalid)\n"); diff --git a/src/test/fuzz/script_assets_test_minimizer.cpp b/src/test/fuzz/script_assets_test_minimizer.cpp index 5f07acbcc7..cec5212f42 100644 --- a/src/test/fuzz/script_assets_test_minimizer.cpp +++ b/src/test/fuzz/script_assets_test_minimizer.cpp @@ -161,7 +161,7 @@ void Test(const std::string& str) tx.vin[idx].scriptWitness = ScriptWitnessFromJSON(test["success"]["witness"]); PrecomputedTransactionData txdata; txdata.Init(tx, std::vector(prevouts)); - MutableTransactionSignatureChecker txcheck(&tx, idx, prevouts[idx].nValue, txdata); + MutableTransactionSignatureChecker txcheck(&tx, idx, prevouts[idx].nValue, txdata, MissingDataBehavior::ASSERT_FAIL); for (const auto flags : ALL_FLAGS) { // "final": true tests are valid for all flags. Others are only valid with flags that are // a subset of test_flags. @@ -176,7 +176,7 @@ void Test(const std::string& str) tx.vin[idx].scriptWitness = ScriptWitnessFromJSON(test["failure"]["witness"]); PrecomputedTransactionData txdata; txdata.Init(tx, std::vector(prevouts)); - MutableTransactionSignatureChecker txcheck(&tx, idx, prevouts[idx].nValue, txdata); + MutableTransactionSignatureChecker txcheck(&tx, idx, prevouts[idx].nValue, txdata, MissingDataBehavior::ASSERT_FAIL); for (const auto flags : ALL_FLAGS) { // If a test is supposed to fail with test_flags, it should also fail with any superset thereof. if ((flags & test_flags) == test_flags) { diff --git a/src/test/fuzz/script_flags.cpp b/src/test/fuzz/script_flags.cpp index 561230707c..b441d5a9d0 100644 --- a/src/test/fuzz/script_flags.cpp +++ b/src/test/fuzz/script_flags.cpp @@ -50,7 +50,7 @@ FUZZ_TARGET_INIT(script_flags, initialize_script_flags) for (unsigned i = 0; i < tx.vin.size(); ++i) { const CTxOut& prevout = txdata.m_spent_outputs.at(i); - const TransactionSignatureChecker checker{&tx, i, prevout.nValue, txdata}; + const TransactionSignatureChecker checker{&tx, i, prevout.nValue, txdata, MissingDataBehavior::ASSERT_FAIL}; ScriptError serror; const bool ret = VerifyScript(tx.vin.at(i).scriptSig, prevout.scriptPubKey, &tx.vin.at(i).scriptWitness, verify_flags, checker, &serror); diff --git a/src/test/multisig_tests.cpp b/src/test/multisig_tests.cpp index e14d2dd72d..39f9b7ee28 100644 --- a/src/test/multisig_tests.cpp +++ b/src/test/multisig_tests.cpp @@ -77,20 +77,20 @@ BOOST_AUTO_TEST_CASE(multisig_verify) keys.assign(1,key[0]); keys.push_back(key[1]); s = sign_multisig(a_and_b, keys, CTransaction(txTo[0]), 0); - BOOST_CHECK(VerifyScript(s, a_and_b, nullptr, flags, MutableTransactionSignatureChecker(&txTo[0], 0, amount), &err)); + BOOST_CHECK(VerifyScript(s, a_and_b, nullptr, flags, MutableTransactionSignatureChecker(&txTo[0], 0, amount, MissingDataBehavior::ASSERT_FAIL), &err)); BOOST_CHECK_MESSAGE(err == SCRIPT_ERR_OK, ScriptErrorString(err)); for (int i = 0; i < 4; i++) { keys.assign(1,key[i]); s = sign_multisig(a_and_b, keys, CTransaction(txTo[0]), 0); - BOOST_CHECK_MESSAGE(!VerifyScript(s, a_and_b, nullptr, flags, MutableTransactionSignatureChecker(&txTo[0], 0, amount), &err), strprintf("a&b 1: %d", i)); + BOOST_CHECK_MESSAGE(!VerifyScript(s, a_and_b, nullptr, flags, MutableTransactionSignatureChecker(&txTo[0], 0, amount, MissingDataBehavior::ASSERT_FAIL), &err), strprintf("a&b 1: %d", i)); BOOST_CHECK_MESSAGE(err == SCRIPT_ERR_INVALID_STACK_OPERATION, ScriptErrorString(err)); keys.assign(1,key[1]); keys.push_back(key[i]); s = sign_multisig(a_and_b, keys, CTransaction(txTo[0]), 0); - BOOST_CHECK_MESSAGE(!VerifyScript(s, a_and_b, nullptr, flags, MutableTransactionSignatureChecker(&txTo[0], 0, amount), &err), strprintf("a&b 2: %d", i)); + BOOST_CHECK_MESSAGE(!VerifyScript(s, a_and_b, nullptr, flags, MutableTransactionSignatureChecker(&txTo[0], 0, amount, MissingDataBehavior::ASSERT_FAIL), &err), strprintf("a&b 2: %d", i)); BOOST_CHECK_MESSAGE(err == SCRIPT_ERR_EVAL_FALSE, ScriptErrorString(err)); } @@ -101,18 +101,18 @@ BOOST_AUTO_TEST_CASE(multisig_verify) s = sign_multisig(a_or_b, keys, CTransaction(txTo[1]), 0); if (i == 0 || i == 1) { - BOOST_CHECK_MESSAGE(VerifyScript(s, a_or_b, nullptr, flags, MutableTransactionSignatureChecker(&txTo[1], 0, amount), &err), strprintf("a|b: %d", i)); + BOOST_CHECK_MESSAGE(VerifyScript(s, a_or_b, nullptr, flags, MutableTransactionSignatureChecker(&txTo[1], 0, amount, MissingDataBehavior::ASSERT_FAIL), &err), strprintf("a|b: %d", i)); BOOST_CHECK_MESSAGE(err == SCRIPT_ERR_OK, ScriptErrorString(err)); } else { - BOOST_CHECK_MESSAGE(!VerifyScript(s, a_or_b, nullptr, flags, MutableTransactionSignatureChecker(&txTo[1], 0, amount), &err), strprintf("a|b: %d", i)); + BOOST_CHECK_MESSAGE(!VerifyScript(s, a_or_b, nullptr, flags, MutableTransactionSignatureChecker(&txTo[1], 0, amount, MissingDataBehavior::ASSERT_FAIL), &err), strprintf("a|b: %d", i)); BOOST_CHECK_MESSAGE(err == SCRIPT_ERR_EVAL_FALSE, ScriptErrorString(err)); } } s.clear(); s << OP_0 << OP_1; - BOOST_CHECK(!VerifyScript(s, a_or_b, nullptr, flags, MutableTransactionSignatureChecker(&txTo[1], 0, amount), &err)); + BOOST_CHECK(!VerifyScript(s, a_or_b, nullptr, flags, MutableTransactionSignatureChecker(&txTo[1], 0, amount, MissingDataBehavior::ASSERT_FAIL), &err)); BOOST_CHECK_MESSAGE(err == SCRIPT_ERR_SIG_DER, ScriptErrorString(err)); @@ -124,12 +124,12 @@ BOOST_AUTO_TEST_CASE(multisig_verify) s = sign_multisig(escrow, keys, CTransaction(txTo[2]), 0); if (i < j && i < 3 && j < 3) { - BOOST_CHECK_MESSAGE(VerifyScript(s, escrow, nullptr, flags, MutableTransactionSignatureChecker(&txTo[2], 0, amount), &err), strprintf("escrow 1: %d %d", i, j)); + BOOST_CHECK_MESSAGE(VerifyScript(s, escrow, nullptr, flags, MutableTransactionSignatureChecker(&txTo[2], 0, amount, MissingDataBehavior::ASSERT_FAIL), &err), strprintf("escrow 1: %d %d", i, j)); BOOST_CHECK_MESSAGE(err == SCRIPT_ERR_OK, ScriptErrorString(err)); } else { - BOOST_CHECK_MESSAGE(!VerifyScript(s, escrow, nullptr, flags, MutableTransactionSignatureChecker(&txTo[2], 0, amount), &err), strprintf("escrow 2: %d %d", i, j)); + BOOST_CHECK_MESSAGE(!VerifyScript(s, escrow, nullptr, flags, MutableTransactionSignatureChecker(&txTo[2], 0, amount, MissingDataBehavior::ASSERT_FAIL), &err), strprintf("escrow 2: %d %d", i, j)); BOOST_CHECK_MESSAGE(err == SCRIPT_ERR_EVAL_FALSE, ScriptErrorString(err)); } } diff --git a/src/test/script_p2sh_tests.cpp b/src/test/script_p2sh_tests.cpp index 856ec6346d..d8a44a65dd 100644 --- a/src/test/script_p2sh_tests.cpp +++ b/src/test/script_p2sh_tests.cpp @@ -41,7 +41,7 @@ Verify(const CScript& scriptSig, const CScript& scriptPubKey, bool fStrict, Scri txTo.vin[0].scriptSig = scriptSig; txTo.vout[0].nValue = 1; - return VerifyScript(scriptSig, scriptPubKey, nullptr, fStrict ? SCRIPT_VERIFY_P2SH : SCRIPT_VERIFY_NONE, MutableTransactionSignatureChecker(&txTo, 0, txFrom.vout[0].nValue), &err); + return VerifyScript(scriptSig, scriptPubKey, nullptr, fStrict ? SCRIPT_VERIFY_P2SH : SCRIPT_VERIFY_NONE, MutableTransactionSignatureChecker(&txTo, 0, txFrom.vout[0].nValue, MissingDataBehavior::ASSERT_FAIL), &err); } diff --git a/src/test/script_tests.cpp b/src/test/script_tests.cpp index 25ca171b33..14cfadf75d 100644 --- a/src/test/script_tests.cpp +++ b/src/test/script_tests.cpp @@ -135,7 +135,7 @@ void DoTest(const CScript& scriptPubKey, const CScript& scriptSig, const CScript const CTransaction txCredit{BuildCreditingTransaction(scriptPubKey, nValue)}; CMutableTransaction tx = BuildSpendingTransaction(scriptSig, scriptWitness, txCredit); CMutableTransaction tx2 = tx; - BOOST_CHECK_MESSAGE(VerifyScript(scriptSig, scriptPubKey, &scriptWitness, flags, MutableTransactionSignatureChecker(&tx, 0, txCredit.vout[0].nValue), &err) == expect, message); + BOOST_CHECK_MESSAGE(VerifyScript(scriptSig, scriptPubKey, &scriptWitness, flags, MutableTransactionSignatureChecker(&tx, 0, txCredit.vout[0].nValue, MissingDataBehavior::ASSERT_FAIL), &err) == expect, message); BOOST_CHECK_MESSAGE(err == scriptError, FormatScriptError(err) + " where " + FormatScriptError((ScriptError_t)scriptError) + " expected: " + message); // Verify that removing flags from a passing test or adding flags to a failing test does not change the result. @@ -145,7 +145,7 @@ void DoTest(const CScript& scriptPubKey, const CScript& scriptSig, const CScript // Weed out some invalid flag combinations. if (combined_flags & SCRIPT_VERIFY_CLEANSTACK && ~combined_flags & (SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_WITNESS)) continue; if (combined_flags & SCRIPT_VERIFY_WITNESS && ~combined_flags & SCRIPT_VERIFY_P2SH) continue; - BOOST_CHECK_MESSAGE(VerifyScript(scriptSig, scriptPubKey, &scriptWitness, combined_flags, MutableTransactionSignatureChecker(&tx, 0, txCredit.vout[0].nValue), &err) == expect, message + strprintf(" (with flags %x)", combined_flags)); + BOOST_CHECK_MESSAGE(VerifyScript(scriptSig, scriptPubKey, &scriptWitness, combined_flags, MutableTransactionSignatureChecker(&tx, 0, txCredit.vout[0].nValue, MissingDataBehavior::ASSERT_FAIL), &err) == expect, message + strprintf(" (with flags %x)", combined_flags)); } #if defined(HAVE_CONSENSUS_LIB) @@ -1071,18 +1071,18 @@ BOOST_AUTO_TEST_CASE(script_CHECKMULTISIG12) CMutableTransaction txTo12 = BuildSpendingTransaction(CScript(), CScriptWitness(), txFrom12); CScript goodsig1 = sign_multisig(scriptPubKey12, key1, CTransaction(txTo12)); - BOOST_CHECK(VerifyScript(goodsig1, scriptPubKey12, nullptr, gFlags, MutableTransactionSignatureChecker(&txTo12, 0, txFrom12.vout[0].nValue), &err)); + BOOST_CHECK(VerifyScript(goodsig1, scriptPubKey12, nullptr, gFlags, MutableTransactionSignatureChecker(&txTo12, 0, txFrom12.vout[0].nValue, MissingDataBehavior::ASSERT_FAIL), &err)); BOOST_CHECK_MESSAGE(err == SCRIPT_ERR_OK, ScriptErrorString(err)); txTo12.vout[0].nValue = 2; - BOOST_CHECK(!VerifyScript(goodsig1, scriptPubKey12, nullptr, gFlags, MutableTransactionSignatureChecker(&txTo12, 0, txFrom12.vout[0].nValue), &err)); + BOOST_CHECK(!VerifyScript(goodsig1, scriptPubKey12, nullptr, gFlags, MutableTransactionSignatureChecker(&txTo12, 0, txFrom12.vout[0].nValue, MissingDataBehavior::ASSERT_FAIL), &err)); BOOST_CHECK_MESSAGE(err == SCRIPT_ERR_EVAL_FALSE, ScriptErrorString(err)); CScript goodsig2 = sign_multisig(scriptPubKey12, key2, CTransaction(txTo12)); - BOOST_CHECK(VerifyScript(goodsig2, scriptPubKey12, nullptr, gFlags, MutableTransactionSignatureChecker(&txTo12, 0, txFrom12.vout[0].nValue), &err)); + BOOST_CHECK(VerifyScript(goodsig2, scriptPubKey12, nullptr, gFlags, MutableTransactionSignatureChecker(&txTo12, 0, txFrom12.vout[0].nValue, MissingDataBehavior::ASSERT_FAIL), &err)); BOOST_CHECK_MESSAGE(err == SCRIPT_ERR_OK, ScriptErrorString(err)); CScript badsig1 = sign_multisig(scriptPubKey12, key3, CTransaction(txTo12)); - BOOST_CHECK(!VerifyScript(badsig1, scriptPubKey12, nullptr, gFlags, MutableTransactionSignatureChecker(&txTo12, 0, txFrom12.vout[0].nValue), &err)); + BOOST_CHECK(!VerifyScript(badsig1, scriptPubKey12, nullptr, gFlags, MutableTransactionSignatureChecker(&txTo12, 0, txFrom12.vout[0].nValue, MissingDataBehavior::ASSERT_FAIL), &err)); BOOST_CHECK_MESSAGE(err == SCRIPT_ERR_EVAL_FALSE, ScriptErrorString(err)); } @@ -1104,54 +1104,54 @@ BOOST_AUTO_TEST_CASE(script_CHECKMULTISIG23) std::vector keys; keys.push_back(key1); keys.push_back(key2); CScript goodsig1 = sign_multisig(scriptPubKey23, keys, CTransaction(txTo23)); - BOOST_CHECK(VerifyScript(goodsig1, scriptPubKey23, nullptr, gFlags, MutableTransactionSignatureChecker(&txTo23, 0, txFrom23.vout[0].nValue), &err)); + BOOST_CHECK(VerifyScript(goodsig1, scriptPubKey23, nullptr, gFlags, MutableTransactionSignatureChecker(&txTo23, 0, txFrom23.vout[0].nValue, MissingDataBehavior::ASSERT_FAIL), &err)); BOOST_CHECK_MESSAGE(err == SCRIPT_ERR_OK, ScriptErrorString(err)); keys.clear(); keys.push_back(key1); keys.push_back(key3); CScript goodsig2 = sign_multisig(scriptPubKey23, keys, CTransaction(txTo23)); - BOOST_CHECK(VerifyScript(goodsig2, scriptPubKey23, nullptr, gFlags, MutableTransactionSignatureChecker(&txTo23, 0, txFrom23.vout[0].nValue), &err)); + BOOST_CHECK(VerifyScript(goodsig2, scriptPubKey23, nullptr, gFlags, MutableTransactionSignatureChecker(&txTo23, 0, txFrom23.vout[0].nValue, MissingDataBehavior::ASSERT_FAIL), &err)); BOOST_CHECK_MESSAGE(err == SCRIPT_ERR_OK, ScriptErrorString(err)); keys.clear(); keys.push_back(key2); keys.push_back(key3); CScript goodsig3 = sign_multisig(scriptPubKey23, keys, CTransaction(txTo23)); - BOOST_CHECK(VerifyScript(goodsig3, scriptPubKey23, nullptr, gFlags, MutableTransactionSignatureChecker(&txTo23, 0, txFrom23.vout[0].nValue), &err)); + BOOST_CHECK(VerifyScript(goodsig3, scriptPubKey23, nullptr, gFlags, MutableTransactionSignatureChecker(&txTo23, 0, txFrom23.vout[0].nValue, MissingDataBehavior::ASSERT_FAIL), &err)); BOOST_CHECK_MESSAGE(err == SCRIPT_ERR_OK, ScriptErrorString(err)); keys.clear(); keys.push_back(key2); keys.push_back(key2); // Can't re-use sig CScript badsig1 = sign_multisig(scriptPubKey23, keys, CTransaction(txTo23)); - BOOST_CHECK(!VerifyScript(badsig1, scriptPubKey23, nullptr, gFlags, MutableTransactionSignatureChecker(&txTo23, 0, txFrom23.vout[0].nValue), &err)); + BOOST_CHECK(!VerifyScript(badsig1, scriptPubKey23, nullptr, gFlags, MutableTransactionSignatureChecker(&txTo23, 0, txFrom23.vout[0].nValue, MissingDataBehavior::ASSERT_FAIL), &err)); BOOST_CHECK_MESSAGE(err == SCRIPT_ERR_EVAL_FALSE, ScriptErrorString(err)); keys.clear(); keys.push_back(key2); keys.push_back(key1); // sigs must be in correct order CScript badsig2 = sign_multisig(scriptPubKey23, keys, CTransaction(txTo23)); - BOOST_CHECK(!VerifyScript(badsig2, scriptPubKey23, nullptr, gFlags, MutableTransactionSignatureChecker(&txTo23, 0, txFrom23.vout[0].nValue), &err)); + BOOST_CHECK(!VerifyScript(badsig2, scriptPubKey23, nullptr, gFlags, MutableTransactionSignatureChecker(&txTo23, 0, txFrom23.vout[0].nValue, MissingDataBehavior::ASSERT_FAIL), &err)); BOOST_CHECK_MESSAGE(err == SCRIPT_ERR_EVAL_FALSE, ScriptErrorString(err)); keys.clear(); keys.push_back(key3); keys.push_back(key2); // sigs must be in correct order CScript badsig3 = sign_multisig(scriptPubKey23, keys, CTransaction(txTo23)); - BOOST_CHECK(!VerifyScript(badsig3, scriptPubKey23, nullptr, gFlags, MutableTransactionSignatureChecker(&txTo23, 0, txFrom23.vout[0].nValue), &err)); + BOOST_CHECK(!VerifyScript(badsig3, scriptPubKey23, nullptr, gFlags, MutableTransactionSignatureChecker(&txTo23, 0, txFrom23.vout[0].nValue, MissingDataBehavior::ASSERT_FAIL), &err)); BOOST_CHECK_MESSAGE(err == SCRIPT_ERR_EVAL_FALSE, ScriptErrorString(err)); keys.clear(); keys.push_back(key4); keys.push_back(key2); // sigs must match pubkeys CScript badsig4 = sign_multisig(scriptPubKey23, keys, CTransaction(txTo23)); - BOOST_CHECK(!VerifyScript(badsig4, scriptPubKey23, nullptr, gFlags, MutableTransactionSignatureChecker(&txTo23, 0, txFrom23.vout[0].nValue), &err)); + BOOST_CHECK(!VerifyScript(badsig4, scriptPubKey23, nullptr, gFlags, MutableTransactionSignatureChecker(&txTo23, 0, txFrom23.vout[0].nValue, MissingDataBehavior::ASSERT_FAIL), &err)); BOOST_CHECK_MESSAGE(err == SCRIPT_ERR_EVAL_FALSE, ScriptErrorString(err)); keys.clear(); keys.push_back(key1); keys.push_back(key4); // sigs must match pubkeys CScript badsig5 = sign_multisig(scriptPubKey23, keys, CTransaction(txTo23)); - BOOST_CHECK(!VerifyScript(badsig5, scriptPubKey23, nullptr, gFlags, MutableTransactionSignatureChecker(&txTo23, 0, txFrom23.vout[0].nValue), &err)); + BOOST_CHECK(!VerifyScript(badsig5, scriptPubKey23, nullptr, gFlags, MutableTransactionSignatureChecker(&txTo23, 0, txFrom23.vout[0].nValue, MissingDataBehavior::ASSERT_FAIL), &err)); BOOST_CHECK_MESSAGE(err == SCRIPT_ERR_EVAL_FALSE, ScriptErrorString(err)); keys.clear(); // Must have signatures CScript badsig6 = sign_multisig(scriptPubKey23, keys, CTransaction(txTo23)); - BOOST_CHECK(!VerifyScript(badsig6, scriptPubKey23, nullptr, gFlags, MutableTransactionSignatureChecker(&txTo23, 0, txFrom23.vout[0].nValue), &err)); + BOOST_CHECK(!VerifyScript(badsig6, scriptPubKey23, nullptr, gFlags, MutableTransactionSignatureChecker(&txTo23, 0, txFrom23.vout[0].nValue, MissingDataBehavior::ASSERT_FAIL), &err)); BOOST_CHECK_MESSAGE(err == SCRIPT_ERR_INVALID_STACK_OPERATION, ScriptErrorString(err)); } diff --git a/src/test/sigopcount_tests.cpp b/src/test/sigopcount_tests.cpp index 7e5274450d..12fc575c1e 100644 --- a/src/test/sigopcount_tests.cpp +++ b/src/test/sigopcount_tests.cpp @@ -71,7 +71,7 @@ static ScriptError VerifyWithFlag(const CTransaction& output, const CMutableTran { ScriptError error; CTransaction inputi(input); - bool ret = VerifyScript(inputi.vin[0].scriptSig, output.vout[0].scriptPubKey, &inputi.vin[0].scriptWitness, flags, TransactionSignatureChecker(&inputi, 0, output.vout[0].nValue), &error); + bool ret = VerifyScript(inputi.vin[0].scriptSig, output.vout[0].scriptPubKey, &inputi.vin[0].scriptWitness, flags, TransactionSignatureChecker(&inputi, 0, output.vout[0].nValue, MissingDataBehavior::ASSERT_FAIL), &error); BOOST_CHECK((ret == true) == (error == SCRIPT_ERR_OK)); return error; diff --git a/src/test/transaction_tests.cpp b/src/test/transaction_tests.cpp index 8f1d99b199..aaa6caa4f1 100644 --- a/src/test/transaction_tests.cpp +++ b/src/test/transaction_tests.cpp @@ -108,7 +108,7 @@ bool CheckTxScripts(const CTransaction& tx, const std::map& const CAmount amount = map_prevout_values.count(input.prevout) ? map_prevout_values.at(input.prevout) : 0; try { tx_valid = VerifyScript(input.scriptSig, map_prevout_scriptPubKeys.at(input.prevout), - &input.scriptWitness, flags, TransactionSignatureChecker(&tx, i, amount, txdata), &err); + &input.scriptWitness, flags, TransactionSignatureChecker(&tx, i, amount, txdata, MissingDataBehavior::ASSERT_FAIL), &err); } catch (...) { BOOST_ERROR("Bad test: " << strTest); return true; // The test format is bad and an error is thrown. Return true to silence further error. @@ -427,7 +427,7 @@ static void CheckWithFlag(const CTransactionRef& output, const CMutableTransacti { ScriptError error; CTransaction inputi(input); - bool ret = VerifyScript(inputi.vin[0].scriptSig, output->vout[0].scriptPubKey, &inputi.vin[0].scriptWitness, flags, TransactionSignatureChecker(&inputi, 0, output->vout[0].nValue), &error); + bool ret = VerifyScript(inputi.vin[0].scriptSig, output->vout[0].scriptPubKey, &inputi.vin[0].scriptWitness, flags, TransactionSignatureChecker(&inputi, 0, output->vout[0].nValue, MissingDataBehavior::ASSERT_FAIL), &error); assert(ret == success); } -- cgit v1.2.3 From 497718b467330b2c6bb0d44786020c55f1aa75f9 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Mon, 1 Mar 2021 18:10:13 -0800 Subject: Treat amount<0 also as missing data for P2WPKH/P2WSH Historically lack of amount data has been treated as amount==-1. Change this and treat it as missing data, as introduced in the previous commits. To be minimally invasive, do this at SignatureHash() call sites rather than inside SignatureHash() (which currently has no means or returning a failure code). --- src/script/interpreter.cpp | 3 +++ src/script/sign.cpp | 3 +++ 2 files changed, 6 insertions(+) (limited to 'src') diff --git a/src/script/interpreter.cpp b/src/script/interpreter.cpp index 2b10f5b0df..abc0625bb1 100644 --- a/src/script/interpreter.cpp +++ b/src/script/interpreter.cpp @@ -1681,6 +1681,9 @@ bool GenericTransactionSignatureChecker::CheckECDSASignature(const std::vecto int nHashType = vchSig.back(); vchSig.pop_back(); + // Witness sighashes need the amount. + if (sigversion == SigVersion::WITNESS_V0 && amount < 0) return HandleMissingData(m_mdb); + uint256 sighash = SignatureHash(scriptCode, *txTo, nIn, nHashType, amount, sigversion, this->txdata); if (!VerifyECDSASignature(vchSig, pubkey, sighash)) diff --git a/src/script/sign.cpp b/src/script/sign.cpp index fb3466f320..6426c8de59 100644 --- a/src/script/sign.cpp +++ b/src/script/sign.cpp @@ -26,6 +26,9 @@ bool MutableTransactionSignatureCreator::CreateSig(const SigningProvider& provid if (sigversion == SigVersion::WITNESS_V0 && !key.IsCompressed()) return false; + // Signing for witness scripts needs the amount. + if (sigversion == SigVersion::WITNESS_V0 && amount < 0) return false; + uint256 hash = SignatureHash(scriptCode, *txTo, nIn, nHashType, amount, sigversion); if (!key.Sign(hash, vchSig)) return false; -- cgit v1.2.3 From 725d7ae0494d4a45f5a840bbbd19c008a7363965 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Mon, 1 Mar 2021 18:14:47 -0800 Subject: Use PrecomputedTransactionData in signet check This is out of an abundance of caution only, as signet currently doesn't enable taproot validation flags. Still, it seems cleaner to make sure that all non-test code that passes MissingDataBehavior::ASSERT_FAIL also actually makes sure no data can be missing. --- src/signet.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'src') diff --git a/src/signet.cpp b/src/signet.cpp index e41b94da8d..368e16f9e7 100644 --- a/src/signet.cpp +++ b/src/signet.cpp @@ -139,7 +139,9 @@ bool CheckSignetBlockSolution(const CBlock& block, const Consensus::Params& cons const CScript& scriptSig = signet_txs->m_to_sign.vin[0].scriptSig; const CScriptWitness& witness = signet_txs->m_to_sign.vin[0].scriptWitness; - TransactionSignatureChecker sigcheck(&signet_txs->m_to_sign, /*nIn=*/ 0, /*amount=*/ signet_txs->m_to_spend.vout[0].nValue, MissingDataBehavior::ASSERT_FAIL); + PrecomputedTransactionData txdata; + txdata.Init(signet_txs->m_to_sign, {signet_txs->m_to_spend.vout[0]}); + TransactionSignatureChecker sigcheck(&signet_txs->m_to_sign, /*nIn=*/ 0, /*amount=*/ signet_txs->m_to_spend.vout[0].nValue, txdata, MissingDataBehavior::ASSERT_FAIL); if (!VerifyScript(scriptSig, signet_txs->m_to_spend.vout[0].scriptPubKey, &witness, BLOCK_SCRIPT_VERIFY_FLAGS, sigcheck)) { LogPrint(BCLog::VALIDATION, "CheckSignetBlockSolution: Errors in block (block solution invalid)\n"); -- cgit v1.2.3