From ae4151bbad74ab54de818d7704fa4568ee65e40d Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Sat, 25 Oct 2014 05:24:14 -0700 Subject: No semantic change: reuse stack variable in P2SH evaluation --- src/script/interpreter.cpp | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) (limited to 'src/script') diff --git a/src/script/interpreter.cpp b/src/script/interpreter.cpp index 5eda23731d..d97f917c37 100644 --- a/src/script/interpreter.cpp +++ b/src/script/interpreter.cpp @@ -1107,21 +1107,24 @@ bool VerifyScript(const CScript& scriptSig, const CScript& scriptPubKey, unsigne if (!scriptSig.IsPushOnly()) return set_error(serror, SCRIPT_ERR_SIG_PUSHONLY); - // stackCopy cannot be empty here, because if it was the + // Restore stack. + swap(stack, stackCopy); + + // stack cannot be empty here, because if it was the // P2SH HASH <> EQUAL scriptPubKey would be evaluated with // an empty stack and the EvalScript above would return false. - assert(!stackCopy.empty()); + assert(!stack.empty()); - const valtype& pubKeySerialized = stackCopy.back(); + const valtype& pubKeySerialized = stack.back(); CScript pubKey2(pubKeySerialized.begin(), pubKeySerialized.end()); - popstack(stackCopy); + popstack(stack); - if (!EvalScript(stackCopy, pubKey2, flags, checker, serror)) + if (!EvalScript(stack, pubKey2, flags, checker, serror)) // serror is set return false; - if (stackCopy.empty()) + if (stack.empty()) return set_error(serror, SCRIPT_ERR_EVAL_FALSE); - if (!CastToBool(stackCopy.back())) + if (!CastToBool(stack.back())) return set_error(serror, SCRIPT_ERR_EVAL_FALSE); else return set_success(serror); -- cgit v1.2.3 From b6e03cc59208305681745ad06f2056ffe6690597 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Sun, 12 Oct 2014 18:39:47 -0700 Subject: 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 | 10 ++++++++-- src/script/script_error.h | 1 + 3 files changed, 21 insertions(+), 5 deletions(-) (limited to 'src/script') diff --git a/src/script/interpreter.cpp b/src/script/interpreter.cpp index d97f917c37..1b187f5ef5 100644 --- a/src/script/interpreter.cpp +++ b/src/script/interpreter.cpp @@ -1096,7 +1096,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); @@ -1126,8 +1125,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 35b2f6c65a..9b35b176ae 100644 --- a/src/script/interpreter.h +++ b/src/script/interpreter.h @@ -67,8 +67,14 @@ enum // discouraged NOPs fails the script. This verification flag will never be // a mandatory flag applied to scripts in a block. NOPs that are not // executed, e.g. within an unexecuted IF ENDIF block, are *not* rejected. - SCRIPT_VERIFY_DISCOURAGE_UPGRADABLE_NOPS = (1U << 7) - + 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), }; uint256 SignatureHash(const CScript &scriptCode, const CTransaction& txTo, unsigned int nIn, int nHashType); diff --git a/src/script/script_error.h b/src/script/script_error.h index ac1f2deae5..f085b63032 100644 --- a/src/script/script_error.h +++ b/src/script/script_error.h @@ -43,6 +43,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, -- cgit v1.2.3 From da918ac06e0d064e9584959ab3d241d03500e972 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Wed, 19 Nov 2014 15:52:50 +0100 Subject: Make SCRIPT_VERIFY_CLEANSTACK a standardness requirement --- src/script/standard.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'src/script') diff --git a/src/script/standard.h b/src/script/standard.h index c4b82b4c45..ed85816a37 100644 --- a/src/script/standard.h +++ b/src/script/standard.h @@ -48,7 +48,8 @@ static const unsigned int STANDARD_SCRIPT_VERIFY_FLAGS = MANDATORY_SCRIPT_VERIFY SCRIPT_VERIFY_STRICTENC | SCRIPT_VERIFY_MINIMALDATA | SCRIPT_VERIFY_NULLDUMMY | - SCRIPT_VERIFY_DISCOURAGE_UPGRADABLE_NOPS; + SCRIPT_VERIFY_DISCOURAGE_UPGRADABLE_NOPS | + SCRIPT_VERIFY_CLEANSTACK; /** For convenience, standard but not mandatory verify flags. */ static const unsigned int STANDARD_NOT_MANDATORY_VERIFY_FLAGS = STANDARD_SCRIPT_VERIFY_FLAGS & ~MANDATORY_SCRIPT_VERIFY_FLAGS; -- cgit v1.2.3