diff --git a/src/core_write.cpp b/src/core_write.cpp --- a/src/core_write.cpp +++ b/src/core_write.cpp @@ -116,7 +116,8 @@ // won't decode correctly formatted public keys in Pubkey or // Multisig scripts due to the restrictions on the pubkey // formats (see IsCompressedOrUncompressedPubKey) being - // incongruous with the checks in CheckSignatureEncoding. + // incongruous with the checks in + // CheckTransactionSignatureEncoding. uint32_t flags = SCRIPT_VERIFY_STRICTENC; if (vch.back() & SIGHASH_FORKID) { // If the transaction is using SIGHASH_FORKID, we need @@ -124,7 +125,8 @@ // TODO: Remove after the Hard Fork. flags |= SCRIPT_ENABLE_SIGHASH_FORKID; } - if (CheckSignatureEncoding(vch, flags, nullptr)) { + if (CheckTransactionSignatureEncoding(vch, flags, + nullptr)) { const uint8_t chSigHashType = vch.back(); if (mapSigHashTypes.count(chSigHashType)) { strSigHashDecode = diff --git a/src/script/interpreter.cpp b/src/script/interpreter.cpp --- a/src/script/interpreter.cpp +++ b/src/script/interpreter.cpp @@ -876,7 +876,8 @@ valtype &vchSig = stacktop(-2); valtype &vchPubKey = stacktop(-1); - if (!CheckSignatureEncoding(vchSig, flags, serror) || + if (!CheckTransactionSignatureEncoding(vchSig, flags, + serror) || !CheckPubKeyEncoding(vchPubKey, flags, serror)) { // serror is set return false; @@ -974,8 +975,8 @@ // 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) || + if (!CheckTransactionSignatureEncoding( + vchSig, flags, serror) || !CheckPubKeyEncoding(vchPubKey, flags, serror)) { // serror is set diff --git a/src/script/sigencoding.h b/src/script/sigencoding.h --- a/src/script/sigencoding.h +++ b/src/script/sigencoding.h @@ -27,8 +27,17 @@ } // namespace -bool CheckSignatureEncoding(const valtype &vchSig, uint32_t flags, - ScriptError *serror); +/** + * Check that the signature provided to authentify a transaction is properly + * encoded. Signatures passed to OP_CHECKSIG, OP_CHECKMULTISIG and their verify + * variants must be checked using this function. + */ +bool CheckTransactionSignatureEncoding(const valtype &vchSig, uint32_t flags, + ScriptError *serror); + +/** + * Check that a public key is encoded properly. + */ bool CheckPubKeyEncoding(const valtype &vchPubKey, uint32_t flags, ScriptError *serror); diff --git a/src/script/sigencoding.cpp b/src/script/sigencoding.cpp --- a/src/script/sigencoding.cpp +++ b/src/script/sigencoding.cpp @@ -11,6 +11,8 @@ #include +typedef boost::sliced_range slicedvaltype; + /** * A canonical signature exists of: <30> <02> <02> , where R and S are not negative (their first byte has its @@ -22,8 +24,7 @@ * * This function is consensus-critical since BIP66. */ -static bool -IsValidSignatureEncoding(const boost::sliced_range &sig) { +static bool IsValidSignatureEncoding(const slicedvaltype &sig) { // Format: 0x30 [total-length] 0x02 [R-length] [R] 0x02 [S-length] [S] // * total-length: 1-byte length descriptor of everything that follows, // excluding the sighash byte. @@ -152,46 +153,52 @@ return true; } -static bool IsLowDERSignature(const valtype &vchSig, ScriptError *serror) { - assert(vchSig.size() > 0); - if (CPubKey::CheckLowS(vchSig | - boost::adaptors::sliced(0, vchSig.size() - 1))) { - return true; +static bool CheckRawSignatureEncoding(const slicedvaltype &sig, uint32_t flags, + ScriptError *serror) { + if ((flags & (SCRIPT_VERIFY_DERSIG | SCRIPT_VERIFY_LOW_S | + SCRIPT_VERIFY_STRICTENC)) && + !IsValidSignatureEncoding(sig)) { + return set_error(serror, SCRIPT_ERR_SIG_DER); } - return set_error(serror, SCRIPT_ERR_SIG_HIGH_S); + + if ((flags & SCRIPT_VERIFY_LOW_S) && !CPubKey::CheckLowS(sig)) { + return set_error(serror, SCRIPT_ERR_SIG_HIGH_S); + } + + return true; } -bool CheckSignatureEncoding(const valtype &vchSig, uint32_t flags, - ScriptError *serror) { +bool CheckTransactionSignatureEncoding(const valtype &vchSig, uint32_t flags, + ScriptError *serror) { // Empty signature. Not strictly DER encoded, but allowed to provide a // compact way to provide an invalid signature for use with CHECK(MULTI)SIG if (vchSig.size() == 0) { return true; } - if ((flags & (SCRIPT_VERIFY_DERSIG | SCRIPT_VERIFY_LOW_S | - SCRIPT_VERIFY_STRICTENC)) != 0 && - !IsValidSignatureEncoding( - vchSig | boost::adaptors::sliced(0, vchSig.size() - 1))) { - return set_error(serror, SCRIPT_ERR_SIG_DER); - } - if ((flags & SCRIPT_VERIFY_LOW_S) != 0 && - !IsLowDERSignature(vchSig, serror)) { + + if (!CheckRawSignatureEncoding( + vchSig | boost::adaptors::sliced(0, vchSig.size() - 1), flags, + serror)) { // serror is set return false; } - if ((flags & SCRIPT_VERIFY_STRICTENC) != 0) { + + if (flags & SCRIPT_VERIFY_STRICTENC) { if (!GetHashType(vchSig).isDefined()) { return set_error(serror, SCRIPT_ERR_SIG_HASHTYPE); } + bool usesForkId = GetHashType(vchSig).hasForkId(); bool forkIdEnabled = flags & SCRIPT_ENABLE_SIGHASH_FORKID; if (!forkIdEnabled && usesForkId) { return set_error(serror, SCRIPT_ERR_ILLEGAL_FORKID); } + if (forkIdEnabled && !usesForkId) { return set_error(serror, SCRIPT_ERR_MUST_USE_FORKID); } } + return true; } diff --git a/src/test/sigencoding_tests.cpp b/src/test/sigencoding_tests.cpp --- a/src/test/sigencoding_tests.cpp +++ b/src/test/sigencoding_tests.cpp @@ -22,7 +22,7 @@ for (int i = 0; i <= 0xff; i++) { ScriptError err = SCRIPT_ERR_OK; valtype sig = SignatureWithHashType(vchSig, SigHashType(i)); - BOOST_CHECK(!CheckSignatureEncoding(sig, flags, &err)); + BOOST_CHECK(!CheckTransactionSignatureEncoding(sig, flags, &err)); BOOST_CHECK_EQUAL(err, expected_error); } } @@ -48,7 +48,7 @@ // Check the signature with the proper forkid flag. SigHashType sigHash = baseSigHash.withForkId(hasForkId); valtype validSig = SignatureWithHashType(vchSig, sigHash); - BOOST_CHECK(CheckSignatureEncoding(validSig, flags, &err)); + BOOST_CHECK(CheckTransactionSignatureEncoding(validSig, flags, &err)); // If we have strict encoding, we prevent the use of undefined flags. std::array undefSigHashes{ @@ -58,8 +58,9 @@ for (SigHashType undefSigHash : undefSigHashes) { valtype undefSighash = SignatureWithHashType(vchSig, undefSigHash); - BOOST_CHECK_EQUAL(CheckSignatureEncoding(undefSighash, flags, &err), - !hasStrictEnc); + BOOST_CHECK_EQUAL( + CheckTransactionSignatureEncoding(undefSighash, flags, &err), + !hasStrictEnc); if (hasStrictEnc) { BOOST_CHECK_EQUAL(err, SCRIPT_ERR_SIG_HASHTYPE); } @@ -69,8 +70,9 @@ SigHashType invalidSigHash = baseSigHash.withForkId(!hasForkId); valtype invalidSig = SignatureWithHashType(vchSig, invalidSigHash); - BOOST_CHECK_EQUAL(CheckSignatureEncoding(invalidSig, flags, &err), - !hasStrictEnc); + BOOST_CHECK_EQUAL( + CheckTransactionSignatureEncoding(invalidSig, flags, &err), + !hasStrictEnc); if (hasStrictEnc) { BOOST_CHECK_EQUAL(err, hasForkId ? SCRIPT_ERR_MUST_USE_FORKID @@ -170,7 +172,7 @@ ScriptError err = SCRIPT_ERR_OK; // Empty sig is always valid. - BOOST_CHECK(CheckSignatureEncoding({}, flags, &err)); + BOOST_CHECK(CheckTransactionSignatureEncoding({}, flags, &err)); // Signature are valid as long as the forkid flag is correct. CheckSignatureEncodingWithSigHashType(minimalSig, flags);