From 00802bb21d882886a86c19104722385ea9ff2bb6 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kittywhiskers@users.noreply.github.com> Date: Wed, 5 Apr 2023 21:48:02 +0000 Subject: [PATCH] partial bitcoin#17938: Disallow automatic conversion between disparate hash types includes: - 0a5ea32ce605984094c5552877cb99bc81654f2c - 3fcc46812334074d2c77a6233e8a961cd0785872 - 2c54217f913967703b404747133be67cf2f4feac - 966a22d859db37b1775e2180e5be032fc4fdf483 - 4d7369125a82214ea42b808a32b71b315a5c3c72 --- src/evo/deterministicmns.cpp | 4 +- src/qt/coincontroldialog.cpp | 2 +- src/rpc/evo.cpp | 6 +-- src/rpc/util.cpp | 2 +- src/script/sign.cpp | 2 +- src/script/standard.cpp | 14 ++++-- src/script/standard.h | 86 +++++++++++++++++++++++++++++----- src/spork.cpp | 2 +- src/wallet/rpcdump.cpp | 4 +- src/wallet/rpcwallet.cpp | 4 +- src/wallet/scriptpubkeyman.cpp | 6 +-- 11 files changed, 102 insertions(+), 30 deletions(-) diff --git a/src/evo/deterministicmns.cpp b/src/evo/deterministicmns.cpp index f91214ccea..1c8e57ed4f 100644 --- a/src/evo/deterministicmns.cpp +++ b/src/evo/deterministicmns.cpp @@ -1416,7 +1416,7 @@ template static bool CheckHashSig(const ProTx& proTx, const PKHash& pkhash, CValidationState& state) { std::string strError; - if (!CHashSigner::VerifyHash(::SerializeHash(proTx), CKeyID(pkhash), proTx.vchSig, strError)) { + if (!CHashSigner::VerifyHash(::SerializeHash(proTx), ToKeyID(pkhash), proTx.vchSig, strError)) { return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-protx-sig"); } return true; @@ -1426,7 +1426,7 @@ template static bool CheckStringSig(const ProTx& proTx, const PKHash& pkhash, CValidationState& state) { std::string strError; - if (!CMessageSigner::VerifyMessage(CKeyID(pkhash), proTx.vchSig, proTx.MakeSignString(), strError)) { + if (!CMessageSigner::VerifyMessage(ToKeyID(pkhash), proTx.vchSig, proTx.MakeSignString(), strError)) { return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-protx-sig"); } return true; diff --git a/src/qt/coincontroldialog.cpp b/src/qt/coincontroldialog.cpp index 88958b8759..4f4dd56a6b 100644 --- a/src/qt/coincontroldialog.cpp +++ b/src/qt/coincontroldialog.cpp @@ -512,7 +512,7 @@ void CoinControlDialog::updateLabels(CCoinControl& m_coin_control, WalletModel * { CPubKey pubkey; PKHash *pkhash = std::get_if(&address); - if (pkhash && model->wallet().getPubKey(out.txout.scriptPubKey, CKeyID(*pkhash), pubkey)) + if (pkhash && model->wallet().getPubKey(out.txout.scriptPubKey, ToKeyID(*pkhash), pubkey)) { nBytesInputs += (pubkey.IsCompressed() ? 148 : 180); } diff --git a/src/rpc/evo.cpp b/src/rpc/evo.cpp index 90df3dff89..58dff88ebc 100644 --- a/src/rpc/evo.cpp +++ b/src/rpc/evo.cpp @@ -178,7 +178,7 @@ static CKeyID ParsePubKeyIDFromAddress(const std::string& strAddress, const std: if (!pkhash) { throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("%s must be a valid P2PKH address, not %s", paramName, strAddress)); } - return CKeyID(*pkhash); + return ToKeyID(*pkhash); } static CBLSPublicKey ParseBLSPubKey(const std::string& hexKey, const std::string& paramName, bool specific_legacy_bls_scheme = false) @@ -773,7 +773,7 @@ static UniValue protx_register_common_wrapper(const JSONRPCRequest& request, } CKey key; - if (!spk_man->GetKey(CKeyID(*pkhash), key)) { + if (!spk_man->GetKey(ToKeyID(*pkhash), key)) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("collateral key not in wallet: %s", EncodeDestination(txDest))); } SignSpecialTxPayloadByString(tx, ptx, key); @@ -1235,7 +1235,7 @@ static bool CheckWalletOwnsScript(CWallet* pwallet, const CScript& script) { CTxDestination dest; if (ExtractDestination(script, dest)) { - if ((std::get_if(&dest) && spk_man->HaveKey(CKeyID(*std::get_if(&dest)))) || (std::get_if(&dest) && spk_man->HaveCScript(ScriptHash(*std::get_if(&dest))))) { + if ((std::get_if(&dest) && spk_man->HaveKey(ToKeyID(*std::get_if(&dest)))) || (std::get_if(&dest) && spk_man->HaveCScript(CScriptID{ScriptHash(*std::get_if(&dest))}))) { return true; } } diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp index 6b8b48a26a..e180354240 100644 --- a/src/rpc/util.cpp +++ b/src/rpc/util.cpp @@ -197,7 +197,7 @@ CPubKey AddrToPubKey(const FillableSigningProvider& keystore, const std::string& throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("%s does not refer to a key", addr_in)); } CPubKey vchPubKey; - if (!keystore.GetPubKey(CKeyID(*pkhash), vchPubKey)) { + if (!keystore.GetPubKey(ToKeyID(*pkhash), vchPubKey)) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("no full public key for address %s", addr_in)); } if (!vchPubKey.IsFullyValid()) { diff --git a/src/script/sign.cpp b/src/script/sign.cpp index 6cbbdfcd8b..063c560660 100644 --- a/src/script/sign.cpp +++ b/src/script/sign.cpp @@ -123,7 +123,7 @@ static bool SignStep(const SigningProvider& provider, const BaseSignatureCreator } case TxoutType::SCRIPTHASH: h160 = uint160(vSolutions[0]); - if (GetCScript(provider, sigdata, h160, scriptRet)) { + if (GetCScript(provider, sigdata, CScriptID{h160}, scriptRet)) { ret.push_back(std::vector(scriptRet.begin(), scriptRet.end())); return true; } diff --git a/src/script/standard.cpp b/src/script/standard.cpp index 228a140c4e..3f7db837d0 100644 --- a/src/script/standard.cpp +++ b/src/script/standard.cpp @@ -13,11 +13,19 @@ typedef std::vector valtype; bool fAcceptDatacarrier = DEFAULT_ACCEPT_DATACARRIER; unsigned nMaxDatacarrierBytes = MAX_OP_RETURN_RELAY; -CScriptID::CScriptID(const CScript& in) : uint160(Hash160(in.begin(), in.end())) {} +CScriptID::CScriptID(const CScript& in) : BaseHash(Hash160(in.begin(), in.end())) {} +CScriptID::CScriptID(const ScriptHash& in) : BaseHash(static_cast(in)) {} -ScriptHash::ScriptHash(const CScript& in) : uint160(Hash160(in.begin(), in.end())) {} +ScriptHash::ScriptHash(const CScript& in) : BaseHash(Hash160(in.begin(), in.end())) {} +ScriptHash::ScriptHash(const CScriptID& in) : BaseHash(static_cast(in)) {} -PKHash::PKHash(const CPubKey& pubkey) : uint160(pubkey.GetID()) {} +PKHash::PKHash(const CPubKey& pubkey) : BaseHash(pubkey.GetID()) {} +PKHash::PKHash(const CKeyID& pubkey_id) : BaseHash(pubkey_id) {} + +CKeyID ToKeyID(const PKHash& key_hash) +{ + return CKeyID{static_cast(key_hash)}; +} const char* GetTxnOutputType(TxoutType t) { diff --git a/src/script/standard.h b/src/script/standard.h index 71439b45d2..1fe7f7b0f8 100644 --- a/src/script/standard.h +++ b/src/script/standard.h @@ -15,14 +15,77 @@ static const bool DEFAULT_ACCEPT_DATACARRIER = true; class CKeyID; class CScript; +struct ScriptHash; + +template +class BaseHash +{ +protected: + HashType m_hash; + +public: + BaseHash() : m_hash() {} + BaseHash(const HashType& in) : m_hash(in) {} + + unsigned char* begin() + { + return m_hash.begin(); + } + + const unsigned char* begin() const + { + return m_hash.begin(); + } + + unsigned char* end() + { + return m_hash.end(); + } + + const unsigned char* end() const + { + return m_hash.end(); + } + + operator std::vector() const + { + return std::vector{m_hash.begin(), m_hash.end()}; + } + + std::string ToString() const + { + return m_hash.ToString(); + } + + bool operator==(const BaseHash& other) const noexcept + { + return m_hash == other.m_hash; + } + + bool operator!=(const BaseHash& other) const noexcept + { + return !(m_hash == other.m_hash); + } + + bool operator<(const BaseHash& other) const noexcept + { + return m_hash < other.m_hash; + } + + size_t size() const + { + return m_hash.size(); + } +}; /** A reference to a CScript: the Hash160 of its serialization (see script.h) */ -class CScriptID : public uint160 +class CScriptID : public BaseHash { public: - CScriptID() : uint160() {} + CScriptID() : BaseHash() {} explicit CScriptID(const CScript& in); - CScriptID(const uint160& in) : uint160(in) {} + explicit CScriptID(const uint160& in) : BaseHash(in) {} + explicit CScriptID(const ScriptHash& in); }; /** @@ -67,20 +130,21 @@ public: friend bool operator<(const CNoDestination &a, const CNoDestination &b) { return true; } }; -struct PKHash : public uint160 +struct PKHash : public BaseHash { - PKHash() : uint160() {} - explicit PKHash(const uint160& hash) : uint160(hash) {} + PKHash() : BaseHash() {} + explicit PKHash(const uint160& hash) : BaseHash(hash) {} explicit PKHash(const CPubKey& pubkey); - using uint160::uint160; + explicit PKHash(const CKeyID& pubkey_id); }; +CKeyID ToKeyID(const PKHash& key_hash); -struct ScriptHash : public uint160 +struct ScriptHash : public BaseHash { - ScriptHash() : uint160() {} - explicit ScriptHash(const uint160& hash) : uint160(hash) {} + ScriptHash() : BaseHash() {} + explicit ScriptHash(const uint160& hash) : BaseHash(hash) {} explicit ScriptHash(const CScript& script); - using uint160::uint160; + explicit ScriptHash(const CScriptID& script); }; /** diff --git a/src/spork.cpp b/src/spork.cpp index ef27566ea4..2daae12a01 100644 --- a/src/spork.cpp +++ b/src/spork.cpp @@ -286,7 +286,7 @@ bool CSporkManager::SetSporkAddress(const std::string& strAddress) LogPrintf("CSporkManager::SetSporkAddress -- Failed to parse spork address\n"); return false; } - setSporkPubKeyIDs.insert(CKeyID(*pkhash)); + setSporkPubKeyIDs.insert(ToKeyID(*pkhash)); return true; } diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index 8926e9135b..e90071ed0b 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -145,7 +145,7 @@ UniValue importprivkey(const JSONRPCRequest& request) } // Use timestamp of 1 to scan the whole chain - if (!pwallet->ImportPrivKeys({{CKeyID(vchAddress), key}}, 1)) { + if (!pwallet->ImportPrivKeys({{ToKeyID(vchAddress), key}}, 1)) { throw JSONRPCError(RPC_WALLET_ERROR, "Error adding key to wallet"); } } @@ -829,7 +829,7 @@ UniValue dumpprivkey(const JSONRPCRequest& request) throw JSONRPCError(RPC_TYPE_ERROR, "Address does not refer to a key"); } CKey vchSecret; - if (!spk_man.GetKey(CKeyID(*pkhash), vchSecret)) { + if (!spk_man.GetKey(ToKeyID(*pkhash), vchSecret)) { throw JSONRPCError(RPC_WALLET_ERROR, "Private key for address " + strAddress + " is not known"); } return EncodeSecret(vchSecret); diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 454b0a7fd8..6fab10d8f5 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -3564,7 +3564,7 @@ public: UniValue operator()(const CNoDestination &dest) const { return UniValue(UniValue::VOBJ); } UniValue operator()(const PKHash& pkhash) const { - CKeyID keyID(pkhash); + CKeyID keyID{ToKeyID(pkhash)}; UniValue obj(UniValue::VOBJ); CPubKey vchPubKey; if (provider && provider->GetPubKey(keyID, vchPubKey)) { @@ -3727,7 +3727,7 @@ UniValue getaddressinfo(const JSONRPCRequest& request) CHDChain hdChainCurrent; LegacyScriptPubKeyMan* legacy_spk_man = pwallet->GetLegacyScriptPubKeyMan(); if (legacy_spk_man != nullptr) { - if (pkhash && legacy_spk_man->HaveHDKey(CKeyID(*pkhash), hdChainCurrent)) { + if (pkhash && legacy_spk_man->HaveHDKey(ToKeyID(*pkhash), hdChainCurrent)) { ret.pushKV("hdchainid", hdChainCurrent.GetID().GetHex()); } } diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp index b947842532..bc3b6c7c32 100644 --- a/src/wallet/scriptpubkeyman.cpp +++ b/src/wallet/scriptpubkeyman.cpp @@ -685,7 +685,7 @@ bool LegacyScriptPubKeyMan::SignTransaction(CMutableTransaction& tx, const std:: SigningResult LegacyScriptPubKeyMan::SignMessage(const std::string& message, const PKHash& pkhash, std::string& str_sig) const { - CKeyID key_id(pkhash); + CKeyID key_id(ToKeyID(pkhash)); CKey key; if (!GetKey(key_id, key)) { return SigningResult::PRIVATE_KEY_NOT_AVAILABLE; @@ -744,8 +744,8 @@ const CKeyMetadata* LegacyScriptPubKeyMan::GetMetadata(const CTxDestination& des LOCK(cs_KeyStore); const PKHash *pkhash = std::get_if(&dest); - if (pkhash != nullptr && !pkhash->IsNull()) { - auto it = mapKeyMetadata.find(CKeyID(*pkhash)); + if (pkhash != nullptr && !ToKeyID(*pkhash).IsNull()) { + auto it = mapKeyMetadata.find(ToKeyID(*pkhash)); if (it != mapKeyMetadata.end()) { return &it->second; }