From 98b135f97f16005687f420136114f80555bc8688 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Sat, 8 Nov 2014 09:32:29 -0800 Subject: Make STRICTENC invalid pubkeys fail the script rather than the opcode. This turns STRICTENC turn into a softforking-safe change (even though it is not intended as a consensus rule), and as a result guarantee that using it for mempool validation only results in consensus-valid transactions in the mempool. --- src/script/interpreter.cpp | 12 ++++++------ src/script/interpreter.h | 4 ++-- src/script/script_error.cpp | 2 ++ src/script/script_error.h | 1 + 4 files changed, 11 insertions(+), 8 deletions(-) (limited to 'src/script') diff --git a/src/script/interpreter.cpp b/src/script/interpreter.cpp index 760086eab..a2a2edce6 100644 --- a/src/script/interpreter.cpp +++ b/src/script/interpreter.cpp @@ -207,9 +207,9 @@ bool static CheckSignatureEncoding(const valtype &vchSig, unsigned int flags, Sc return true; } -bool static CheckPubKeyEncoding(const valtype &vchSig, unsigned int flags) { +bool static CheckPubKeyEncoding(const valtype &vchSig, unsigned int flags, ScriptError* serror) { if ((flags & SCRIPT_VERIFY_STRICTENC) != 0 && !IsCompressedOrUncompressedPubKey(vchSig)) { - return false; + return set_error(serror, SCRIPT_ERR_PUBKEYTYPE); } return true; } @@ -792,11 +792,11 @@ bool EvalScript(vector >& stack, const CScript& script, un // Drop the signature, since there's no way for a signature to sign itself scriptCode.FindAndDelete(CScript(vchSig)); - if (!CheckSignatureEncoding(vchSig, flags, serror)) { + if (!CheckSignatureEncoding(vchSig, flags, serror) || !CheckPubKeyEncoding(vchPubKey, flags, serror)) { //serror is set return false; } - bool fSuccess = CheckPubKeyEncoding(vchPubKey, flags) && checker.CheckSig(vchSig, vchPubKey, scriptCode); + bool fSuccess = checker.CheckSig(vchSig, vchPubKey, scriptCode); popstack(stack); popstack(stack); @@ -855,13 +855,13 @@ bool EvalScript(vector >& stack, const CScript& script, un valtype& vchSig = stacktop(-isig); valtype& vchPubKey = stacktop(-ikey); - if (!CheckSignatureEncoding(vchSig, flags, serror)) { + if (!CheckSignatureEncoding(vchSig, flags, serror) || !CheckPubKeyEncoding(vchPubKey, flags, serror)) { // serror is set return false; } // Check signature - bool fOk = CheckPubKeyEncoding(vchPubKey, flags) && checker.CheckSig(vchSig, vchPubKey, scriptCode); + bool fOk = checker.CheckSig(vchSig, vchPubKey, scriptCode); if (fOk) { isig++; diff --git a/src/script/interpreter.h b/src/script/interpreter.h index 12b271941..35b2f6c65 100644 --- a/src/script/interpreter.h +++ b/src/script/interpreter.h @@ -35,8 +35,8 @@ enum SCRIPT_VERIFY_P2SH = (1U << 0), // Passing a non-strict-DER signature or one with undefined hashtype to a checksig operation causes script failure. - // Passing a pubkey that is not (0x04 + 64 bytes) or (0x02 or 0x03 + 32 bytes) to checksig causes that pubkey to be - // skipped (not softfork safe: this flag can widen the validity of OP_CHECKSIG OP_NOT). + // Evaluating a pubkey that is not (0x04 + 64 bytes) or (0x02 or 0x03 + 32 bytes) by checksig causes script failure. + // (softfork safe, but not used or intended as a consensus rule). SCRIPT_VERIFY_STRICTENC = (1U << 1), // Passing a non-strict-DER signature to a checksig operation causes script failure (softfork safe, BIP62 rule 1) diff --git a/src/script/script_error.cpp b/src/script/script_error.cpp index 793fc0da4..5d24ed98b 100644 --- a/src/script/script_error.cpp +++ b/src/script/script_error.cpp @@ -61,6 +61,8 @@ const char* ScriptErrorString(const ScriptError serror) return "Dummy CHECKMULTISIG argument must be zero"; case SCRIPT_ERR_DISCOURAGE_UPGRADABLE_NOPS: return "NOPx reserved for soft-fork upgrades"; + case SCRIPT_ERR_PUBKEYTYPE: + return "Public key is neither compressed or uncompressed"; case SCRIPT_ERR_UNKNOWN_ERROR: case SCRIPT_ERR_ERROR_COUNT: default: break; diff --git a/src/script/script_error.h b/src/script/script_error.h index 21153f1bd..ac1f2deae 100644 --- a/src/script/script_error.h +++ b/src/script/script_error.h @@ -42,6 +42,7 @@ typedef enum ScriptError_t SCRIPT_ERR_SIG_PUSHONLY, SCRIPT_ERR_SIG_HIGH_S, SCRIPT_ERR_SIG_NULLDUMMY, + SCRIPT_ERR_PUBKEYTYPE, /* softfork safeness */ SCRIPT_ERR_DISCOURAGE_UPGRADABLE_NOPS, -- cgit v1.2.3 From ca8158719b17ebdcf1de1e26079b6b896122d0e5 Mon Sep 17 00:00:00 2001 From: Peter Todd Date: Mon, 10 Nov 2014 02:33:19 -0500 Subject: Test the exact order of CHECKMULTISIG sig/pubkey evaluation Possible with STRICTENC --- src/script/interpreter.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'src/script') diff --git a/src/script/interpreter.cpp b/src/script/interpreter.cpp index a2a2edce6..5eda23731 100644 --- a/src/script/interpreter.cpp +++ b/src/script/interpreter.cpp @@ -855,6 +855,9 @@ bool EvalScript(vector >& stack, const CScript& script, un valtype& vchSig = stacktop(-isig); valtype& vchPubKey = stacktop(-ikey); + // Note how this makes the exact order of pubkey/signature evaluation + // distinguishable by CHECKMULTISIG NOT if the STRICTENC flag is set. + // See the script_(in)valid tests for details. if (!CheckSignatureEncoding(vchSig, flags, serror) || !CheckPubKeyEncoding(vchPubKey, flags, serror)) { // serror is set return false; @@ -871,7 +874,8 @@ bool EvalScript(vector >& stack, const CScript& script, un nKeysCount--; // If there are more signatures left than keys left, - // then too many signatures have failed + // then too many signatures have failed. Exit early, + // without checking any further signatures. if (nSigsCount > nKeysCount) fSuccess = false; } -- cgit v1.2.3