From e41bd449ab2b8d01260795383af2c40b659d8587 Mon Sep 17 00:00:00 2001 From: Johnson Lau Date: Thu, 22 Sep 2016 15:06:54 +0800 Subject: [PATCH] Add policy: null signature for failed CHECK(MULTI)SIG --- src/policy/policy.h | 1 + src/script/interpreter.cpp | 14 +++++++++++++- src/script/interpreter.h | 4 ++++ src/script/script_error.cpp | 2 ++ src/script/script_error.h | 1 + src/test/data/script_tests.json | 33 ++++++++++++++++++++++++++++++++- src/test/script_tests.cpp | 1 + src/test/transaction_tests.cpp | 1 + 8 files changed, 55 insertions(+), 2 deletions(-) diff --git a/src/policy/policy.h b/src/policy/policy.h index 458ec4a0c..9d6ff1233 100644 --- a/src/policy/policy.h +++ b/src/policy/policy.h @@ -43,6 +43,7 @@ static const unsigned int STANDARD_SCRIPT_VERIFY_FLAGS = MANDATORY_SCRIPT_VERIFY SCRIPT_VERIFY_DISCOURAGE_UPGRADABLE_NOPS | SCRIPT_VERIFY_CLEANSTACK | SCRIPT_VERIFY_MINIMALIF | + SCRIPT_VERIFY_NULLFAIL | SCRIPT_VERIFY_CHECKLOCKTIMEVERIFY | SCRIPT_VERIFY_CHECKSEQUENCEVERIFY | SCRIPT_VERIFY_LOW_S | diff --git a/src/script/interpreter.cpp b/src/script/interpreter.cpp index fd356fed0..41756ea71 100644 --- a/src/script/interpreter.cpp +++ b/src/script/interpreter.cpp @@ -885,6 +885,9 @@ bool EvalScript(vector >& stack, const CScript& script, un } bool fSuccess = checker.CheckSig(vchSig, vchPubKey, scriptCode, sigversion); + if (!fSuccess && (flags & SCRIPT_VERIFY_NULLFAIL) && vchSig.size()) + return set_error(serror, SCRIPT_ERR_SIG_NULLFAIL); + popstack(stack); popstack(stack); stack.push_back(fSuccess ? vchTrue : vchFalse); @@ -914,6 +917,9 @@ bool EvalScript(vector >& stack, const CScript& script, un if (nOpCount > MAX_OPS_PER_SCRIPT) return set_error(serror, SCRIPT_ERR_OP_COUNT); int ikey = ++i; + // ikey2 is the position of last non-signature item in the stack. Top stack item = 1. + // With SCRIPT_VERIFY_NULLFAIL, this is used for cleanup if operation fails. + int ikey2 = nKeysCount + 2; i += nKeysCount; if ((int)stack.size() < i) return set_error(serror, SCRIPT_ERR_INVALID_STACK_OPERATION); @@ -970,8 +976,14 @@ bool EvalScript(vector >& stack, const CScript& script, un } // Clean up stack of actual arguments - while (i-- > 1) + while (i-- > 1) { + // If the operation failed, we require that all signatures must be empty vector + if (!fSuccess && (flags & SCRIPT_VERIFY_NULLFAIL) && !ikey2 && stacktop(-1).size()) + return set_error(serror, SCRIPT_ERR_SIG_NULLFAIL); + if (ikey2 > 0) + ikey2--; popstack(stack); + } // A bug causes CHECKMULTISIG to consume one extra argument // whose contents were not checked in any way. diff --git a/src/script/interpreter.h b/src/script/interpreter.h index 2ce4b23e5..0adc9482f 100644 --- a/src/script/interpreter.h +++ b/src/script/interpreter.h @@ -98,6 +98,10 @@ enum // Segwit script only: Require the argument of OP_IF/NOTIF to be exactly 0x01 or empty vector // SCRIPT_VERIFY_MINIMALIF = (1U << 13), + + // Signature(s) must be empty vector if an CHECK(MULTI)SIG operation failed + // + SCRIPT_VERIFY_NULLFAIL = (1U << 14), }; bool CheckSignatureEncoding(const std::vector &vchSig, unsigned int flags, ScriptError* serror); diff --git a/src/script/script_error.cpp b/src/script/script_error.cpp index 9969c232f..e27b715c2 100644 --- a/src/script/script_error.cpp +++ b/src/script/script_error.cpp @@ -65,6 +65,8 @@ const char* ScriptErrorString(const ScriptError serror) return "Dummy CHECKMULTISIG argument must be zero"; case SCRIPT_ERR_MINIMALIF: return "OP_IF/NOTIF argument must be minimal"; + case SCRIPT_ERR_SIG_NULLFAIL: + return "Signature must be zero for failed CHECK(MULTI)SIG operation"; case SCRIPT_ERR_DISCOURAGE_UPGRADABLE_NOPS: return "NOPx reserved for soft-fork upgrades"; case SCRIPT_ERR_DISCOURAGE_UPGRADABLE_WITNESS_PROGRAM: diff --git a/src/script/script_error.h b/src/script/script_error.h index 6d34d3792..bccfdb99e 100644 --- a/src/script/script_error.h +++ b/src/script/script_error.h @@ -49,6 +49,7 @@ typedef enum ScriptError_t SCRIPT_ERR_PUBKEYTYPE, SCRIPT_ERR_CLEANSTACK, SCRIPT_ERR_MINIMALIF, + SCRIPT_ERR_SIG_NULLFAIL, /* softfork safeness */ SCRIPT_ERR_DISCOURAGE_UPGRADABLE_NOPS, diff --git a/src/test/data/script_tests.json b/src/test/data/script_tests.json index d456a8bef..06103ea5b 100644 --- a/src/test/data/script_tests.json +++ b/src/test/data/script_tests.json @@ -1491,6 +1491,27 @@ "OK", "BIP66 example 4, with DERSIG" ], +[ + "0x09 0x300602010102010101", + "0x21 0x038282263212c609d9ea2a6e3e172de238d8c39cabd5ac1ca10646e23fd5f51508 CHECKSIG NOT", + "DERSIG", + "OK", + "BIP66 example 4, with DERSIG, non-null DER-compliant signature" +], +[ + "0", + "0x21 0x038282263212c609d9ea2a6e3e172de238d8c39cabd5ac1ca10646e23fd5f51508 CHECKSIG NOT", + "DERSIG,NULLFAIL", + "OK", + "BIP66 example 4, with DERSIG and NULLFAIL" +], +[ + "0x09 0x300602010102010101", + "0x21 0x038282263212c609d9ea2a6e3e172de238d8c39cabd5ac1ca10646e23fd5f51508 CHECKSIG NOT", + "DERSIG,NULLFAIL", + "NULLFAIL", + "BIP66 example 4, with DERSIG and NULLFAIL, non-null DER-compliant signature" +], [ "1", "0x21 0x038282263212c609d9ea2a6e3e172de238d8c39cabd5ac1ca10646e23fd5f51508 CHECKSIG", @@ -2208,5 +2229,15 @@ [["645168", 0.00000001], "0x22 0x0020f913eacf2e38a5d6fc3a8311d72ae704cb83866350a984dd3e5eb76d2a8c28e8", "HASH160 0x14 0xdbb7d1c0a56b7a9c423300c8cca6e6e065baf1dc EQUAL", "P2SH,WITNESS", "UNBALANCED_CONDITIONAL"], [["645168", 0.00000001], "0x22 0x0020f913eacf2e38a5d6fc3a8311d72ae704cb83866350a984dd3e5eb76d2a8c28e8", "HASH160 0x14 0xdbb7d1c0a56b7a9c423300c8cca6e6e065baf1dc EQUAL", "P2SH,WITNESS,MINIMALIF", "UNBALANCED_CONDITIONAL"], - ["The End"] +["NULLFAIL should cover all signatures and signatures only"], +["0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0", "0x01 0x14 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 0x01 0x14 CHECKMULTISIG NOT", "DERSIG", "OK", "BIP66 and NULLFAIL-compliant"], +["0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0", "0x01 0x14 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 0x01 0x14 CHECKMULTISIG NOT", "DERSIG,NULLFAIL", "OK", "BIP66 and NULLFAIL-compliant"], +["1 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0", "0x01 0x14 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 0x01 0x14 CHECKMULTISIG NOT", "DERSIG,NULLFAIL", "OK", "BIP66 and NULLFAIL-compliant, not NULLDUMMY-compliant"], +["1 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0", "0x01 0x14 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 0x01 0x14 CHECKMULTISIG NOT", "DERSIG,NULLFAIL,NULLDUMMY", "SIG_NULLDUMMY", "BIP66 and NULLFAIL-compliant, not NULLDUMMY-compliant"], +["0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0x09 0x300602010102010101", "0x01 0x14 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 0x01 0x14 CHECKMULTISIG NOT", "DERSIG", "OK", "BIP66-compliant but not NULLFAIL-compliant"], +["0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0x09 0x300602010102010101", "0x01 0x14 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 0x01 0x14 CHECKMULTISIG NOT", "DERSIG,NULLFAIL", "NULLFAIL", "BIP66-compliant but not NULLFAIL-compliant"], +["0 0x09 0x300602010102010101 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0", "0x01 0x14 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 0x01 0x14 CHECKMULTISIG NOT", "DERSIG", "OK", "BIP66-compliant but not NULLFAIL-compliant"], +["0 0x09 0x300602010102010101 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0", "0x01 0x14 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 0x01 0x14 CHECKMULTISIG NOT", "DERSIG,NULLFAIL", "NULLFAIL", "BIP66-compliant but not NULLFAIL-compliant"], + +["The End"] ] diff --git a/src/test/script_tests.cpp b/src/test/script_tests.cpp index ee0f14ee5..7971b5122 100644 --- a/src/test/script_tests.cpp +++ b/src/test/script_tests.cpp @@ -90,6 +90,7 @@ static ScriptErrorDesc script_errors[]={ {SCRIPT_ERR_PUBKEYTYPE, "PUBKEYTYPE"}, {SCRIPT_ERR_CLEANSTACK, "CLEANSTACK"}, {SCRIPT_ERR_MINIMALIF, "MINIMALIF"}, + {SCRIPT_ERR_SIG_NULLFAIL, "NULLFAIL"}, {SCRIPT_ERR_DISCOURAGE_UPGRADABLE_NOPS, "DISCOURAGE_UPGRADABLE_NOPS"}, {SCRIPT_ERR_DISCOURAGE_UPGRADABLE_WITNESS_PROGRAM, "DISCOURAGE_UPGRADABLE_WITNESS_PROGRAM"}, {SCRIPT_ERR_WITNESS_PROGRAM_WRONG_LENGTH, "WITNESS_PROGRAM_WRONG_LENGTH"}, diff --git a/src/test/transaction_tests.cpp b/src/test/transaction_tests.cpp index 165dfd9a3..6163d2f63 100644 --- a/src/test/transaction_tests.cpp +++ b/src/test/transaction_tests.cpp @@ -51,6 +51,7 @@ static std::map mapFlagNames = boost::assign::map_list_of (string("DISCOURAGE_UPGRADABLE_NOPS"), (unsigned int)SCRIPT_VERIFY_DISCOURAGE_UPGRADABLE_NOPS) (string("CLEANSTACK"), (unsigned int)SCRIPT_VERIFY_CLEANSTACK) (string("MINIMALIF"), (unsigned int)SCRIPT_VERIFY_MINIMALIF) + (string("NULLFAIL"), (unsigned int)SCRIPT_VERIFY_NULLFAIL) (string("CHECKLOCKTIMEVERIFY"), (unsigned int)SCRIPT_VERIFY_CHECKLOCKTIMEVERIFY) (string("CHECKSEQUENCEVERIFY"), (unsigned int)SCRIPT_VERIFY_CHECKSEQUENCEVERIFY) (string("WITNESS"), (unsigned int)SCRIPT_VERIFY_WITNESS)