From af3208bfa6967d6b35aecf0ba35d9d6bf0f8317e Mon Sep 17 00:00:00 2001 From: mruddy Date: Thu, 30 Jul 2015 19:56:00 -0400 Subject: Resolve issue 3166. These changes decode valid SIGHASH types on signatures in assembly (asm) representations of scriptSig scripts. This squashed commit incorporates substantial helpful feedback from jtimon, laanwj, and sipa. --- src/bitcoin-tx.cpp | 4 +-- src/core_io.h | 4 +-- src/core_write.cpp | 66 ++++++++++++++++++++++++++++++++++++++++-- src/primitives/transaction.cpp | 4 +-- src/rpcrawtransaction.cpp | 8 ++--- src/script/interpreter.cpp | 2 +- src/script/interpreter.h | 2 ++ src/script/script.cpp | 33 --------------------- src/script/script.h | 1 - src/test/script_tests.cpp | 32 +++++++++++++++++++- 10 files changed, 108 insertions(+), 48 deletions(-) (limited to 'src') diff --git a/src/bitcoin-tx.cpp b/src/bitcoin-tx.cpp index e389d51a73..f43df4f331 100644 --- a/src/bitcoin-tx.cpp +++ b/src/bitcoin-tx.cpp @@ -387,8 +387,8 @@ static void MutateTxSign(CMutableTransaction& tx, const string& flagStr) CCoinsModifier coins = view.ModifyCoins(txid); if (coins->IsAvailable(nOut) && coins->vout[nOut].scriptPubKey != scriptPubKey) { string err("Previous output scriptPubKey mismatch:\n"); - err = err + coins->vout[nOut].scriptPubKey.ToString() + "\nvs:\n"+ - scriptPubKey.ToString(); + err = err + ScriptToAsmStr(coins->vout[nOut].scriptPubKey) + "\nvs:\n"+ + ScriptToAsmStr(scriptPubKey); throw runtime_error(err); } if ((unsigned int)nOut >= coins->vout.size()) diff --git a/src/core_io.h b/src/core_io.h index 115e3199dc..ba5b4e6487 100644 --- a/src/core_io.h +++ b/src/core_io.h @@ -16,6 +16,7 @@ class UniValue; // core_read.cpp extern CScript ParseScript(const std::string& s); +extern std::string ScriptToAsmStr(const CScript& script, const bool fAttemptSighashDecode = false); extern bool DecodeHexTx(CTransaction& tx, const std::string& strHexTx); extern bool DecodeHexBlk(CBlock&, const std::string& strHexBlk); extern uint256 ParseHashUV(const UniValue& v, const std::string& strName); @@ -25,8 +26,7 @@ extern std::vector ParseHexUV(const UniValue& v, const std::strin // core_write.cpp extern std::string FormatScript(const CScript& script); extern std::string EncodeHexTx(const CTransaction& tx); -extern void ScriptPubKeyToUniv(const CScript& scriptPubKey, - UniValue& out, bool fIncludeHex); +extern void ScriptPubKeyToUniv(const CScript& scriptPubKey, UniValue& out, bool fIncludeHex); extern void TxToUniv(const CTransaction& tx, const uint256& hashBlock, UniValue& entry); #endif // BITCOIN_CORE_IO_H diff --git a/src/core_write.cpp b/src/core_write.cpp index c3babec2fc..2ad42baddf 100644 --- a/src/core_write.cpp +++ b/src/core_write.cpp @@ -15,6 +15,7 @@ #include "utilmoneystr.h" #include "utilstrencodings.h" +#include #include using namespace std; @@ -54,6 +55,67 @@ string FormatScript(const CScript& script) return ret.substr(0, ret.size() - 1); } +const map mapSigHashTypes = + boost::assign::map_list_of + (static_cast(SIGHASH_ALL), string("ALL")) + (static_cast(SIGHASH_ALL|SIGHASH_ANYONECANPAY), string("ALL|ANYONECANPAY")) + (static_cast(SIGHASH_NONE), string("NONE")) + (static_cast(SIGHASH_NONE|SIGHASH_ANYONECANPAY), string("NONE|ANYONECANPAY")) + (static_cast(SIGHASH_SINGLE), string("SINGLE")) + (static_cast(SIGHASH_SINGLE|SIGHASH_ANYONECANPAY), string("SINGLE|ANYONECANPAY")) + ; + +/** + * Create the assembly string representation of a CScript object. + * @param[in] script CScript object to convert into the asm string representation. + * @param[in] fAttemptSighashDecode Whether to attempt to decode sighash types on data within the script that matches the format + * of a signature. Only pass true for scripts you believe could contain signatures. For example, + * pass false, or omit the this argument (defaults to false), for scriptPubKeys. + */ +string ScriptToAsmStr(const CScript& script, const bool fAttemptSighashDecode) +{ + string str; + opcodetype opcode; + vector vch; + CScript::const_iterator pc = script.begin(); + while (pc < script.end()) { + if (!str.empty()) { + str += " "; + } + if (!script.GetOp(pc, opcode, vch)) { + str += "[error]"; + return str; + } + if (0 <= opcode && opcode <= OP_PUSHDATA4) { + if (vch.size() <= static_cast::size_type>(4)) { + str += strprintf("%d", CScriptNum(vch, false).getint()); + } else { + // the IsUnspendable check makes sure not to try to decode OP_RETURN data that may match the format of a signature + if (fAttemptSighashDecode && !script.IsUnspendable()) { + string strSigHashDecode; + // goal: only attempt to decode a defined sighash type from data that looks like a signature within a scriptSig. + // this 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. + if (CheckSignatureEncoding(vch, SCRIPT_VERIFY_STRICTENC, NULL)) { + const unsigned char chSigHashType = vch.back(); + if (mapSigHashTypes.count(chSigHashType)) { + strSigHashDecode = "[" + mapSigHashTypes.find(chSigHashType)->second + "]"; + vch.pop_back(); // remove the sighash type byte. it will be replaced by the decode. + } + } + str += HexStr(vch) + strSigHashDecode; + } else { + str += HexStr(vch); + } + } + } else { + str += GetOpName(opcode); + } + } + return str; +} + string EncodeHexTx(const CTransaction& tx) { CDataStream ssTx(SER_NETWORK, PROTOCOL_VERSION); @@ -68,7 +130,7 @@ void ScriptPubKeyToUniv(const CScript& scriptPubKey, vector addresses; int nRequired; - out.pushKV("asm", scriptPubKey.ToString()); + out.pushKV("asm", ScriptToAsmStr(scriptPubKey)); if (fIncludeHex) out.pushKV("hex", HexStr(scriptPubKey.begin(), scriptPubKey.end())); @@ -101,7 +163,7 @@ void TxToUniv(const CTransaction& tx, const uint256& hashBlock, UniValue& entry) in.pushKV("txid", txin.prevout.hash.GetHex()); in.pushKV("vout", (int64_t)txin.prevout.n); UniValue o(UniValue::VOBJ); - o.pushKV("asm", txin.scriptSig.ToString()); + o.pushKV("asm", ScriptToAsmStr(txin.scriptSig, true)); o.pushKV("hex", HexStr(txin.scriptSig.begin(), txin.scriptSig.end())); in.pushKV("scriptSig", o); } diff --git a/src/primitives/transaction.cpp b/src/primitives/transaction.cpp index 606dbea798..46d3cbbe2e 100644 --- a/src/primitives/transaction.cpp +++ b/src/primitives/transaction.cpp @@ -36,7 +36,7 @@ std::string CTxIn::ToString() const if (prevout.IsNull()) str += strprintf(", coinbase %s", HexStr(scriptSig)); else - str += strprintf(", scriptSig=%s", scriptSig.ToString().substr(0,24)); + str += strprintf(", scriptSig=%s", HexStr(scriptSig).substr(0, 24)); if (nSequence != std::numeric_limits::max()) str += strprintf(", nSequence=%u", nSequence); str += ")"; @@ -56,7 +56,7 @@ uint256 CTxOut::GetHash() const std::string CTxOut::ToString() const { - return strprintf("CTxOut(nValue=%d.%08d, scriptPubKey=%s)", nValue / COIN, nValue % COIN, scriptPubKey.ToString().substr(0,30)); + return strprintf("CTxOut(nValue=%d.%08d, scriptPubKey=%s)", nValue / COIN, nValue % COIN, HexStr(scriptPubKey).substr(0, 30)); } CMutableTransaction::CMutableTransaction() : nVersion(CTransaction::CURRENT_VERSION), nLockTime(0) {} diff --git a/src/rpcrawtransaction.cpp b/src/rpcrawtransaction.cpp index 62d2ef69ef..6bb14f2b44 100644 --- a/src/rpcrawtransaction.cpp +++ b/src/rpcrawtransaction.cpp @@ -41,7 +41,7 @@ void ScriptPubKeyToJSON(const CScript& scriptPubKey, UniValue& out, bool fInclud vector addresses; int nRequired; - out.push_back(Pair("asm", scriptPubKey.ToString())); + out.push_back(Pair("asm", ScriptToAsmStr(scriptPubKey))); if (fIncludeHex) out.push_back(Pair("hex", HexStr(scriptPubKey.begin(), scriptPubKey.end()))); @@ -73,7 +73,7 @@ void TxToJSON(const CTransaction& tx, const uint256 hashBlock, UniValue& entry) in.push_back(Pair("txid", txin.prevout.hash.GetHex())); in.push_back(Pair("vout", (int64_t)txin.prevout.n)); UniValue o(UniValue::VOBJ); - o.push_back(Pair("asm", txin.scriptSig.ToString())); + o.push_back(Pair("asm", ScriptToAsmStr(txin.scriptSig, true))); o.push_back(Pair("hex", HexStr(txin.scriptSig.begin(), txin.scriptSig.end()))); in.push_back(Pair("scriptSig", o)); } @@ -665,8 +665,8 @@ UniValue signrawtransaction(const UniValue& params, bool fHelp) CCoinsModifier coins = view.ModifyCoins(txid); if (coins->IsAvailable(nOut) && coins->vout[nOut].scriptPubKey != scriptPubKey) { string err("Previous output scriptPubKey mismatch:\n"); - err = err + coins->vout[nOut].scriptPubKey.ToString() + "\nvs:\n"+ - scriptPubKey.ToString(); + err = err + ScriptToAsmStr(coins->vout[nOut].scriptPubKey) + "\nvs:\n"+ + ScriptToAsmStr(scriptPubKey); throw JSONRPCError(RPC_DESERIALIZATION_ERROR, err); } if ((unsigned int)nOut >= coins->vout.size()) diff --git a/src/script/interpreter.cpp b/src/script/interpreter.cpp index 0b78fdf5a8..03af78bce5 100644 --- a/src/script/interpreter.cpp +++ b/src/script/interpreter.cpp @@ -188,7 +188,7 @@ bool static IsDefinedHashtypeSignature(const valtype &vchSig) { return true; } -bool static CheckSignatureEncoding(const valtype &vchSig, unsigned int flags, ScriptError* serror) { +bool CheckSignatureEncoding(const vector &vchSig, unsigned int 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) { diff --git a/src/script/interpreter.h b/src/script/interpreter.h index 35d572f0ad..213e8c7651 100644 --- a/src/script/interpreter.h +++ b/src/script/interpreter.h @@ -83,6 +83,8 @@ enum SCRIPT_VERIFY_CHECKLOCKTIMEVERIFY = (1U << 9), }; +bool CheckSignatureEncoding(const std::vector &vchSig, unsigned int flags, ScriptError* serror); + uint256 SignatureHash(const CScript &scriptCode, const CTransaction& txTo, unsigned int nIn, int nHashType); class BaseSignatureChecker diff --git a/src/script/script.cpp b/src/script/script.cpp index fd33924732..58dbade0e2 100644 --- a/src/script/script.cpp +++ b/src/script/script.cpp @@ -8,16 +8,6 @@ #include "tinyformat.h" #include "utilstrencodings.h" -namespace { -inline std::string ValueString(const std::vector& vch) -{ - if (vch.size() <= 4) - return strprintf("%d", CScriptNum(vch, false).getint()); - else - return HexStr(vch); -} -} // anon namespace - using namespace std; const char* GetOpName(opcodetype opcode) @@ -237,26 +227,3 @@ bool CScript::IsPushOnly() const } return true; } - -std::string CScript::ToString() const -{ - std::string str; - opcodetype opcode; - std::vector vch; - const_iterator pc = begin(); - while (pc < end()) - { - if (!str.empty()) - str += " "; - if (!GetOp(pc, opcode, vch)) - { - str += "[error]"; - return str; - } - if (0 <= opcode && opcode <= OP_PUSHDATA4) - str += ValueString(vch); - else - str += GetOpName(opcode); - } - return str; -} diff --git a/src/script/script.h b/src/script/script.h index e39ca57f4f..f0725bbbf6 100644 --- a/src/script/script.h +++ b/src/script/script.h @@ -601,7 +601,6 @@ public: return (size() > 0 && *begin() == OP_RETURN); } - std::string ToString() const; void clear() { // The default std::vector::clear() does not release memory. diff --git a/src/test/script_tests.cpp b/src/test/script_tests.cpp index 37c046935f..225da0801a 100644 --- a/src/test/script_tests.cpp +++ b/src/test/script_tests.cpp @@ -840,7 +840,7 @@ BOOST_AUTO_TEST_CASE(script_CHECKMULTISIG23) CScript badsig6 = sign_multisig(scriptPubKey23, keys, txTo23); BOOST_CHECK(!VerifyScript(badsig6, scriptPubKey23, flags, MutableTransactionSignatureChecker(&txTo23, 0), &err)); BOOST_CHECK_MESSAGE(err == SCRIPT_ERR_INVALID_STACK_OPERATION, ScriptErrorString(err)); -} +} BOOST_AUTO_TEST_CASE(script_combineSigs) { @@ -983,4 +983,34 @@ BOOST_AUTO_TEST_CASE(script_IsPushOnly_on_invalid_scripts) BOOST_CHECK(!CScript(direct, direct+sizeof(direct)).IsPushOnly()); } +BOOST_AUTO_TEST_CASE(script_GetScriptAsm) +{ + BOOST_CHECK_EQUAL("OP_NOP2", ScriptToAsmStr(CScript() << OP_NOP2, true)); + BOOST_CHECK_EQUAL("OP_NOP2", ScriptToAsmStr(CScript() << OP_CHECKLOCKTIMEVERIFY, true)); + BOOST_CHECK_EQUAL("OP_NOP2", ScriptToAsmStr(CScript() << OP_NOP2)); + BOOST_CHECK_EQUAL("OP_NOP2", ScriptToAsmStr(CScript() << OP_CHECKLOCKTIMEVERIFY)); + + string derSig("304502207fa7a6d1e0ee81132a269ad84e68d695483745cde8b541e3bf630749894e342a022100c1f7ab20e13e22fb95281a870f3dcf38d782e53023ee313d741ad0cfbc0c5090"); + string pubKey("03b0da749730dc9b4b1f4a14d6902877a92541f5368778853d9c4a0cb7802dcfb2"); + vector vchPubKey = ToByteVector(ParseHex(pubKey)); + + BOOST_CHECK_EQUAL(derSig + "00 " + pubKey, ScriptToAsmStr(CScript() << ToByteVector(ParseHex(derSig + "00")) << vchPubKey, true)); + BOOST_CHECK_EQUAL(derSig + "80 " + pubKey, ScriptToAsmStr(CScript() << ToByteVector(ParseHex(derSig + "80")) << vchPubKey, true)); + BOOST_CHECK_EQUAL(derSig + "[ALL] " + pubKey, ScriptToAsmStr(CScript() << ToByteVector(ParseHex(derSig + "01")) << vchPubKey, true)); + BOOST_CHECK_EQUAL(derSig + "[NONE] " + pubKey, ScriptToAsmStr(CScript() << ToByteVector(ParseHex(derSig + "02")) << vchPubKey, true)); + BOOST_CHECK_EQUAL(derSig + "[SINGLE] " + pubKey, ScriptToAsmStr(CScript() << ToByteVector(ParseHex(derSig + "03")) << vchPubKey, true)); + BOOST_CHECK_EQUAL(derSig + "[ALL|ANYONECANPAY] " + pubKey, ScriptToAsmStr(CScript() << ToByteVector(ParseHex(derSig + "81")) << vchPubKey, true)); + BOOST_CHECK_EQUAL(derSig + "[NONE|ANYONECANPAY] " + pubKey, ScriptToAsmStr(CScript() << ToByteVector(ParseHex(derSig + "82")) << vchPubKey, true)); + BOOST_CHECK_EQUAL(derSig + "[SINGLE|ANYONECANPAY] " + pubKey, ScriptToAsmStr(CScript() << ToByteVector(ParseHex(derSig + "83")) << vchPubKey, true)); + + BOOST_CHECK_EQUAL(derSig + "00 " + pubKey, ScriptToAsmStr(CScript() << ToByteVector(ParseHex(derSig + "00")) << vchPubKey)); + BOOST_CHECK_EQUAL(derSig + "80 " + pubKey, ScriptToAsmStr(CScript() << ToByteVector(ParseHex(derSig + "80")) << vchPubKey)); + BOOST_CHECK_EQUAL(derSig + "01 " + pubKey, ScriptToAsmStr(CScript() << ToByteVector(ParseHex(derSig + "01")) << vchPubKey)); + BOOST_CHECK_EQUAL(derSig + "02 " + pubKey, ScriptToAsmStr(CScript() << ToByteVector(ParseHex(derSig + "02")) << vchPubKey)); + BOOST_CHECK_EQUAL(derSig + "03 " + pubKey, ScriptToAsmStr(CScript() << ToByteVector(ParseHex(derSig + "03")) << vchPubKey)); + BOOST_CHECK_EQUAL(derSig + "81 " + pubKey, ScriptToAsmStr(CScript() << ToByteVector(ParseHex(derSig + "81")) << vchPubKey)); + BOOST_CHECK_EQUAL(derSig + "82 " + pubKey, ScriptToAsmStr(CScript() << ToByteVector(ParseHex(derSig + "82")) << vchPubKey)); + BOOST_CHECK_EQUAL(derSig + "83 " + pubKey, ScriptToAsmStr(CScript() << ToByteVector(ParseHex(derSig + "83")) << vchPubKey)); +} + BOOST_AUTO_TEST_SUITE_END() -- cgit v1.2.3