Browse Source

Only pass things committed to by tx's witness hash to CScriptCheck

This clarifies a bit more the ways in which the new script execution
cache could break consensus in the future if additional data from
the CCoins object were to be used as a part of script execution.

After this change, any such consensus breaks should be very visible
to reviewers, hopefully ensuring no such changes can be made.
0.15
Matt Corallo 8 years ago committed by Pieter Wuille
parent
commit
c87b957a32
  1. 3
      src/test/script_P2SH_tests.cpp
  2. 3
      src/test/transaction_tests.cpp
  3. 12
      src/validation.cpp
  4. 4
      src/validation.h

3
src/test/script_P2SH_tests.cpp

@ -112,7 +112,8 @@ BOOST_AUTO_TEST_CASE(sign)
{ {
CScript sigSave = txTo[i].vin[0].scriptSig; CScript sigSave = txTo[i].vin[0].scriptSig;
txTo[i].vin[0].scriptSig = txTo[j].vin[0].scriptSig; txTo[i].vin[0].scriptSig = txTo[j].vin[0].scriptSig;
bool sigOK = CScriptCheck(CCoins(txFrom, 0), txTo[i], 0, SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_STRICTENC, false, &txdata)(); const CTxOut& output = txFrom.vout[txTo[i].vin[0].prevout.n];
bool sigOK = CScriptCheck(output.scriptPubKey, output.nValue, txTo[i], 0, SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_STRICTENC, false, &txdata)();
if (i == j) if (i == j)
BOOST_CHECK_MESSAGE(sigOK, strprintf("VerifySignature %d %d", i, j)); BOOST_CHECK_MESSAGE(sigOK, strprintf("VerifySignature %d %d", i, j));
else else

3
src/test/transaction_tests.cpp

@ -481,7 +481,8 @@ BOOST_AUTO_TEST_CASE(test_big_witness_transaction) {
for(uint32_t i = 0; i < mtx.vin.size(); i++) { for(uint32_t i = 0; i < mtx.vin.size(); i++) {
std::vector<CScriptCheck> vChecks; std::vector<CScriptCheck> vChecks;
CScriptCheck check(coins, tx, i, SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_WITNESS, false, &txdata); const CTxOut& output = coins.vout[tx.vin[i].prevout.n];
CScriptCheck check(output.scriptPubKey, output.nValue, tx, i, SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_WITNESS, false, &txdata);
vChecks.push_back(CScriptCheck()); vChecks.push_back(CScriptCheck());
check.swap(vChecks.back()); check.swap(vChecks.back());
control.Add(vChecks); control.Add(vChecks);

12
src/validation.cpp

@ -1119,8 +1119,16 @@ bool CheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsVi
const CCoins* coins = inputs.AccessCoins(prevout.hash); const CCoins* coins = inputs.AccessCoins(prevout.hash);
assert(coins); assert(coins);
// We very carefully only pass in things to CScriptCheck which
// are clearly committed to by tx' witness hash. This provides
// a sanity check that our caching is not introducing consensus
// failures through additional data in, eg, the coins being
// spent being checked as a part of CScriptCheck.
const CScript& scriptPubKey = coins->vout[prevout.n].scriptPubKey;
const CAmount amount = coins->vout[prevout.n].nValue;
// Verify signature // Verify signature
CScriptCheck check(*coins, tx, i, flags, cacheStore, &txdata); CScriptCheck check(scriptPubKey, amount, tx, i, flags, cacheStore, &txdata);
if (pvChecks) { if (pvChecks) {
pvChecks->push_back(CScriptCheck()); pvChecks->push_back(CScriptCheck());
check.swap(pvChecks->back()); check.swap(pvChecks->back());
@ -1132,7 +1140,7 @@ bool CheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsVi
// arguments; if so, don't trigger DoS protection to // arguments; if so, don't trigger DoS protection to
// avoid splitting the network between upgraded and // avoid splitting the network between upgraded and
// non-upgraded nodes. // non-upgraded nodes.
CScriptCheck check2(*coins, tx, i, CScriptCheck check2(scriptPubKey, amount, tx, i,
flags & ~STANDARD_NOT_MANDATORY_VERIFY_FLAGS, cacheStore, &txdata); flags & ~STANDARD_NOT_MANDATORY_VERIFY_FLAGS, cacheStore, &txdata);
if (check2()) if (check2())
return state.Invalid(false, REJECT_NONSTANDARD, strprintf("non-mandatory-script-verify-flag (%s)", ScriptErrorString(check.GetScriptError()))); return state.Invalid(false, REJECT_NONSTANDARD, strprintf("non-mandatory-script-verify-flag (%s)", ScriptErrorString(check.GetScriptError())));

4
src/validation.h

@ -404,8 +404,8 @@ private:
public: public:
CScriptCheck(): amount(0), ptxTo(0), nIn(0), nFlags(0), cacheStore(false), error(SCRIPT_ERR_UNKNOWN_ERROR) {} CScriptCheck(): amount(0), ptxTo(0), nIn(0), nFlags(0), cacheStore(false), error(SCRIPT_ERR_UNKNOWN_ERROR) {}
CScriptCheck(const CCoins& txFromIn, const CTransaction& txToIn, unsigned int nInIn, unsigned int nFlagsIn, bool cacheIn, PrecomputedTransactionData* txdataIn) : CScriptCheck(const CScript& scriptPubKeyIn, const CAmount amountIn, const CTransaction& txToIn, unsigned int nInIn, unsigned int nFlagsIn, bool cacheIn, PrecomputedTransactionData* txdataIn) :
scriptPubKey(txFromIn.vout[txToIn.vin[nInIn].prevout.n].scriptPubKey), amount(txFromIn.vout[txToIn.vin[nInIn].prevout.n].nValue), scriptPubKey(scriptPubKeyIn), amount(amountIn),
ptxTo(&txToIn), nIn(nInIn), nFlags(nFlagsIn), cacheStore(cacheIn), error(SCRIPT_ERR_UNKNOWN_ERROR), txdata(txdataIn) { } ptxTo(&txToIn), nIn(nInIn), nFlags(nFlagsIn), cacheStore(cacheIn), error(SCRIPT_ERR_UNKNOWN_ERROR), txdata(txdataIn) { }
bool operator()(); bool operator()();

Loading…
Cancel
Save