From 466c8d5be2e68fa4af14130cd4f27df9dece7e1c Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Sun, 12 Oct 2014 18:39:47 -0700 Subject: [PATCH] Add SCRIPT_VERIFY_CLEANSTACK (BIP62 rule 6) Based on an earlier patch by Peter Todd, though the rules here are different (P2SH scripts should not have a CLEANSTACK check before the P2SH evaluation). --- src/script/interpreter.cpp | 15 ++++++++++++--- src/script/interpreter.h | 7 +++++++ src/script/script_error.h | 1 + src/test/data/script_invalid.json | 12 ++++++++++++ src/test/data/script_valid.json | 18 ++++++++++++++++++ src/test/script_tests.cpp | 16 ++++++++++++++++ src/test/transaction_tests.cpp | 1 + 7 files changed, 67 insertions(+), 3 deletions(-) diff --git a/src/script/interpreter.cpp b/src/script/interpreter.cpp index 4e2bfa4345..b4e94fc2c6 100644 --- a/src/script/interpreter.cpp +++ b/src/script/interpreter.cpp @@ -1181,7 +1181,6 @@ bool VerifyScript(const CScript& scriptSig, const CScript& scriptPubKey, unsigne return false; if (stack.empty()) return set_error(serror, SCRIPT_ERR_EVAL_FALSE); - if (CastToBool(stack.back()) == false) return set_error(serror, SCRIPT_ERR_EVAL_FALSE); @@ -1211,8 +1210,18 @@ bool VerifyScript(const CScript& scriptSig, const CScript& scriptPubKey, unsigne return set_error(serror, SCRIPT_ERR_EVAL_FALSE); if (!CastToBool(stack.back())) return set_error(serror, SCRIPT_ERR_EVAL_FALSE); - else - return set_success(serror); + } + + // The CLEANSTACK check is only performed after potential P2SH evaluation, + // as the non-P2SH evaluation of a P2SH script will obviously not result in + // a clean stack (the P2SH inputs remain). + if ((flags & SCRIPT_VERIFY_CLEANSTACK) != 0) { + // Disallow CLEANSTACK without P2SH, as otherwise a switch CLEANSTACK->P2SH+CLEANSTACK + // would be possible, which is not a softfork (and P2SH should be one). + assert((flags & SCRIPT_VERIFY_P2SH) != 0); + if (stack.size() != 1) { + return set_error(serror, SCRIPT_ERR_CLEANSTACK); + } } return set_success(serror); diff --git a/src/script/interpreter.h b/src/script/interpreter.h index 9f76d69a46..8934f0549e 100644 --- a/src/script/interpreter.h +++ b/src/script/interpreter.h @@ -70,6 +70,13 @@ enum // executed, e.g. within an unexecuted IF ENDIF block, are *not* rejected. SCRIPT_VERIFY_DISCOURAGE_UPGRADABLE_NOPS = (1U << 7), + // Require that only a single stack element remains after evaluation. This changes the success criterion from + // "At least one stack element must remain, and when interpreted as a boolean, it must be true" to + // "Exactly one stack element must remain, and when interpreted as a boolean, it must be true". + // (softfork safe, BIP62 rule 6) + // Note: CLEANSTACK should never be used without P2SH. + SCRIPT_VERIFY_CLEANSTACK = (1U << 8), + // Verify CHECKLOCKTIMEVERIFY // // See BIP65 for details. diff --git a/src/script/script_error.h b/src/script/script_error.h index 7b4c40edaa..1113712759 100644 --- a/src/script/script_error.h +++ b/src/script/script_error.h @@ -47,6 +47,7 @@ typedef enum ScriptError_t SCRIPT_ERR_SIG_HIGH_S, SCRIPT_ERR_SIG_NULLDUMMY, SCRIPT_ERR_PUBKEYTYPE, + SCRIPT_ERR_CLEANSTACK, /* softfork safeness */ SCRIPT_ERR_DISCOURAGE_UPGRADABLE_NOPS, diff --git a/src/test/data/script_invalid.json b/src/test/data/script_invalid.json index e63018051d..bbb529147e 100644 --- a/src/test/data/script_invalid.json +++ b/src/test/data/script_invalid.json @@ -773,6 +773,18 @@ "SIGPUSHONLY", "P2SH(P2PK) with non-push scriptSig" ], +[ + "11 0x47 0x3044022057c4ba463d3b8e6848b3896be14c6953caf0528cd390ad15104a109c94b558ba02206fa3922154e1d0bfca92ce5f9adbe4991be33f3f5b7f6bf501e895b7e37fd72f01", + "0x41 0x0479be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798483ada7726a3c4655da4fbfc0e1108a8fd17b448a68554199c47d08ffb10d4b8 CHECKSIG", + "CLEANSTACK,P2SH", + "P2PK with unnecessary input" +], +[ + "11 0x47 0x304402203407c26745ea95ee31fbb5074e730ff9be235d5a8d8e0a2c868358bf6a04797402205cc0e594dfd275583472168298d448be59c2530a4ea4d7c37311b82108f9f8bc01 0x43 0x410479be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798483ada7726a3c4655da4fbfc0e1108a8fd17b448a68554199c47d08ffb10d4b8ac", + "HASH160 0x14 0x31edc23bdafda4639e669f89ad6b2318dd79d032 EQUAL", + "CLEANSTACK,P2SH", + "P2SH with unnecessary input" +], ["The End"] ] diff --git a/src/test/data/script_valid.json b/src/test/data/script_valid.json index 42e9edbd37..d1db172599 100644 --- a/src/test/data/script_valid.json +++ b/src/test/data/script_valid.json @@ -879,6 +879,24 @@ "SIGPUSHONLY", "2-of-2 with two identical keys and sigs pushed" ], +[ + "11 0x47 0x304402204ba8b04dfe8657608427b996bd7c151ff8cd8579b3316c7314549a6c59f6bfb7022058cf052927fbc5e51e26dd4711c470bbf7f3adc8aaaf7bfa304eff6bb6e6399e01", + "0x41 0x0479be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798483ada7726a3c4655da4fbfc0e1108a8fd17b448a68554199c47d08ffb10d4b8 CHECKSIG", + "P2SH", + "P2PK with unnecessary input but no CLEANSTACK" +], +[ + "11 0x47 0x304402202beaa2f6a4ec783091643797f9819b5ae39a03dfcf3b934746e96dd6b2ad5f7202200650a618fb2ce08b4edd160351172e016a041c81622d806390420d15cc6cece401 0x43 0x410479be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798483ada7726a3c4655da4fbfc0e1108a8fd17b448a68554199c47d08ffb10d4b8ac", + "HASH160 0x14 0x31edc23bdafda4639e669f89ad6b2318dd79d032 EQUAL", + "P2SH", + "P2SH with unnecessary input but no CLEANSTACK" +], +[ + "0x47 0x3044022048505fd42afde400932558ea7fa76a52c2fff3130fa820a6a05647f64d5d780e022056a6bc823c95cedf68f6f67ad036533b48b9f3f706355a71f9f6e647e889f79201 0x43 0x410479be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798483ada7726a3c4655da4fbfc0e1108a8fd17b448a68554199c47d08ffb10d4b8ac", + "HASH160 0x14 0x31edc23bdafda4639e669f89ad6b2318dd79d032 EQUAL", + "CLEANSTACK,P2SH", + "P2SH with CLEANSTACK" +], ["The End"] ] diff --git a/src/test/script_tests.cpp b/src/test/script_tests.cpp index a084a145c8..6e73f7f8d4 100644 --- a/src/test/script_tests.cpp +++ b/src/test/script_tests.cpp @@ -563,6 +563,22 @@ BOOST_AUTO_TEST_CASE(script_build) "2-of-2 with two identical keys and sigs pushed", SCRIPT_VERIFY_SIGPUSHONLY ).Num(0).PushSig(keys.key1).PushSig(keys.key1)); + good.push_back(TestBuilder(CScript() << ToByteVector(keys.pubkey0) << OP_CHECKSIG, + "P2PK with unnecessary input but no CLEANSTACK", SCRIPT_VERIFY_P2SH + ).Num(11).PushSig(keys.key0)); + bad.push_back(TestBuilder(CScript() << ToByteVector(keys.pubkey0) << OP_CHECKSIG, + "P2PK with unnecessary input", SCRIPT_VERIFY_CLEANSTACK | SCRIPT_VERIFY_P2SH + ).Num(11).PushSig(keys.key0)); + good.push_back(TestBuilder(CScript() << ToByteVector(keys.pubkey0) << OP_CHECKSIG, + "P2SH with unnecessary input but no CLEANSTACK", SCRIPT_VERIFY_P2SH, true + ).Num(11).PushSig(keys.key0).PushRedeem()); + bad.push_back(TestBuilder(CScript() << ToByteVector(keys.pubkey0) << OP_CHECKSIG, + "P2SH with unnecessary input", SCRIPT_VERIFY_CLEANSTACK | SCRIPT_VERIFY_P2SH, true + ).Num(11).PushSig(keys.key0).PushRedeem()); + good.push_back(TestBuilder(CScript() << ToByteVector(keys.pubkey0) << OP_CHECKSIG, + "P2SH with CLEANSTACK", SCRIPT_VERIFY_CLEANSTACK | SCRIPT_VERIFY_P2SH, true + ).PushSig(keys.key0).PushRedeem()); + std::set tests_good; std::set tests_bad; diff --git a/src/test/transaction_tests.cpp b/src/test/transaction_tests.cpp index 0bdc9bd4c3..56b6d0dae0 100644 --- a/src/test/transaction_tests.cpp +++ b/src/test/transaction_tests.cpp @@ -39,6 +39,7 @@ static std::map mapFlagNames = boost::assign::map_list_of (string("MINIMALDATA"), (unsigned int)SCRIPT_VERIFY_MINIMALDATA) (string("NULLDUMMY"), (unsigned int)SCRIPT_VERIFY_NULLDUMMY) (string("DISCOURAGE_UPGRADABLE_NOPS"), (unsigned int)SCRIPT_VERIFY_DISCOURAGE_UPGRADABLE_NOPS) + (string("CLEANSTACK"), (unsigned int)SCRIPT_VERIFY_CLEANSTACK) (string("CHECKLOCKTIMEVERIFY"), (unsigned int)SCRIPT_VERIFY_CHECKLOCKTIMEVERIFY); unsigned int ParseScriptFlags(string strFlags)