diff --git a/doc/release-notes-30482.md b/doc/release-notes-30482.md new file mode 100644 index 00000000000..fb625c2fa98 --- /dev/null +++ b/doc/release-notes-30482.md @@ -0,0 +1,6 @@ +Updated REST APIs +----------------- +- Parameter validation for `/rest/getutxos` has been improved by rejecting + truncated or overly large txids and malformed outpoint indices by raising an + HTTP_BAD_REQUEST "Parse error". Previously, these malformed requests would be + silently handled. (#30482, #30444) diff --git a/src/bitcoin-tx.cpp b/src/bitcoin-tx.cpp index 89faa0123a5..89c03c16472 100644 --- a/src/bitcoin-tx.cpp +++ b/src/bitcoin-tx.cpp @@ -264,8 +264,8 @@ static void MutateTxAddInput(CMutableTransaction& tx, const std::string& strInpu throw std::runtime_error("TX input missing separator"); // extract and validate TXID - uint256 txid; - if (!ParseHashStr(vStrInputParts[0], txid)) { + auto txid{Txid::FromHex(vStrInputParts[0])}; + if (!txid) { throw std::runtime_error("invalid TX input txid"); } @@ -285,7 +285,7 @@ static void MutateTxAddInput(CMutableTransaction& tx, const std::string& strInpu } // append to transaction input list - CTxIn txin(Txid::FromUint256(txid), vout, CScript(), nSequenceIn); + CTxIn txin(*txid, vout, CScript(), nSequenceIn); tx.vin.push_back(txin); } @@ -625,8 +625,8 @@ static void MutateTxSign(CMutableTransaction& tx, const std::string& flagStr) if (!prevOut.checkObject(types)) throw std::runtime_error("prevtxs internal object typecheck fail"); - uint256 txid; - if (!ParseHashStr(prevOut["txid"].get_str(), txid)) { + auto txid{Txid::FromHex(prevOut["txid"].get_str())}; + if (!txid) { throw std::runtime_error("txid must be hexadecimal string (not '" + prevOut["txid"].get_str() + "')"); } @@ -634,7 +634,7 @@ static void MutateTxSign(CMutableTransaction& tx, const std::string& flagStr) if (nOut < 0) throw std::runtime_error("vout cannot be negative"); - COutPoint out(Txid::FromUint256(txid), nOut); + COutPoint out(*txid, nOut); std::vector pkData(ParseHexUV(prevOut["scriptPubKey"], "scriptPubKey")); CScript scriptPubKey(pkData.begin(), pkData.end()); diff --git a/src/core_io.h b/src/core_io.h index 4405f5c8f83..9305bb72393 100644 --- a/src/core_io.h +++ b/src/core_io.h @@ -37,15 +37,6 @@ std::string ScriptToAsmStr(const CScript& script, const bool fAttemptSighashDeco [[nodiscard]] bool DecodeHexBlk(CBlock&, const std::string& strHexBlk); bool DecodeHexBlockHeader(CBlockHeader&, const std::string& hex_header); -/** - * Parse a hex string into 256 bits - * @param[in] strHex a hex-formatted, 64-character string - * @param[out] result the result of the parsing - * @returns true if successful, false if not - * - * @see ParseHashV for an RPC-oriented version of this - */ -bool ParseHashStr(const std::string& strHex, uint256& result); [[nodiscard]] util::Result SighashFromStr(const std::string& sighash); // core_write.cpp diff --git a/src/core_read.cpp b/src/core_read.cpp index 0ba271a8d29..23f341c2304 100644 --- a/src/core_read.cpp +++ b/src/core_read.cpp @@ -234,15 +234,6 @@ bool DecodeHexBlk(CBlock& block, const std::string& strHexBlk) return true; } -bool ParseHashStr(const std::string& strHex, uint256& result) -{ - if ((strHex.size() != 64) || !IsHex(strHex)) - return false; - - result.SetHex(strHex); - return true; -} - util::Result SighashFromStr(const std::string& sighash) { static const std::map map_sighash_values = { diff --git a/src/qt/transactiontablemodel.cpp b/src/qt/transactiontablemodel.cpp index d4267fcf61f..9214e7723d3 100644 --- a/src/qt/transactiontablemodel.cpp +++ b/src/qt/transactiontablemodel.cpp @@ -277,7 +277,7 @@ void TransactionTableModel::updateAmountColumnTitle() void TransactionTableModel::updateTransaction(const QString &hash, int status, bool showTransaction) { uint256 updated; - updated.SetHex(hash.toStdString()); + updated.SetHexDeprecated(hash.toStdString()); priv->updateWallet(walletModel->wallet(), updated, status, showTransaction); } diff --git a/src/qt/transactionview.cpp b/src/qt/transactionview.cpp index 7e24dbd3ec9..2aaa65c6f73 100644 --- a/src/qt/transactionview.cpp +++ b/src/qt/transactionview.cpp @@ -396,7 +396,7 @@ void TransactionView::contextualMenu(const QPoint &point) // check if transaction can be abandoned, disable context menu action in case it doesn't uint256 hash; - hash.SetHex(selection.at(0).data(TransactionTableModel::TxHashRole).toString().toStdString()); + hash.SetHexDeprecated(selection.at(0).data(TransactionTableModel::TxHashRole).toString().toStdString()); abandonAction->setEnabled(model->wallet().transactionCanBeAbandoned(hash)); bumpFeeAction->setEnabled(model->wallet().transactionCanBeBumped(hash)); copyAddressAction->setEnabled(GUIUtil::hasEntryData(transactionView, 0, TransactionTableModel::AddressRole)); @@ -416,7 +416,7 @@ void TransactionView::abandonTx() // get the hash from the TxHashRole (QVariant / QString) uint256 hash; QString hashQStr = selection.at(0).data(TransactionTableModel::TxHashRole).toString(); - hash.SetHex(hashQStr.toStdString()); + hash.SetHexDeprecated(hashQStr.toStdString()); // Abandon the wallet transaction over the walletModel model->wallet().abandonTransaction(hash); @@ -431,7 +431,7 @@ void TransactionView::bumpFee([[maybe_unused]] bool checked) // get the hash from the TxHashRole (QVariant / QString) uint256 hash; QString hashQStr = selection.at(0).data(TransactionTableModel::TxHashRole).toString(); - hash.SetHex(hashQStr.toStdString()); + hash.SetHexDeprecated(hashQStr.toStdString()); // Bump tx fee over the walletModel uint256 newHash; diff --git a/src/rest.cpp b/src/rest.cpp index 3cf6ad343c8..c42bc8e40c2 100644 --- a/src/rest.cpp +++ b/src/rest.cpp @@ -217,9 +217,10 @@ static bool rest_headers(const std::any& context, return RESTERR(req, HTTP_BAD_REQUEST, strprintf("Header count is invalid or out of acceptable range (1-%u): %s", MAX_REST_HEADERS_RESULTS, raw_count)); } - uint256 hash; - if (!ParseHashStr(hashStr, hash)) + auto hash{uint256::FromHex(hashStr)}; + if (!hash) { return RESTERR(req, HTTP_BAD_REQUEST, "Invalid hash: " + hashStr); + } const CBlockIndex* tip = nullptr; std::vector headers; @@ -231,7 +232,7 @@ static bool rest_headers(const std::any& context, LOCK(cs_main); CChain& active_chain = chainman.ActiveChain(); tip = active_chain.Tip(); - const CBlockIndex* pindex = chainman.m_blockman.LookupBlockIndex(hash); + const CBlockIndex* pindex{chainman.m_blockman.LookupBlockIndex(*hash)}; while (pindex != nullptr && active_chain.Contains(pindex)) { headers.push_back(pindex); if (headers.size() == *parsed_count) { @@ -290,9 +291,10 @@ static bool rest_block(const std::any& context, std::string hashStr; const RESTResponseFormat rf = ParseDataFormat(hashStr, strURIPart); - uint256 hash; - if (!ParseHashStr(hashStr, hash)) + auto hash{uint256::FromHex(hashStr)}; + if (!hash) { return RESTERR(req, HTTP_BAD_REQUEST, "Invalid hash: " + hashStr); + } FlatFilePos pos{}; const CBlockIndex* pblockindex = nullptr; @@ -303,7 +305,7 @@ static bool rest_block(const std::any& context, { LOCK(cs_main); tip = chainman.ActiveChain().Tip(); - pblockindex = chainman.m_blockman.LookupBlockIndex(hash); + pblockindex = chainman.m_blockman.LookupBlockIndex(*hash); if (!pblockindex) { return RESTERR(req, HTTP_NOT_FOUND, hashStr + " not found"); } @@ -390,8 +392,8 @@ static bool rest_filter_header(const std::any& context, HTTPRequest* req, const return RESTERR(req, HTTP_BAD_REQUEST, strprintf("Header count is invalid or out of acceptable range (1-%u): %s", MAX_REST_HEADERS_RESULTS, raw_count)); } - uint256 block_hash; - if (!ParseHashStr(raw_blockhash, block_hash)) { + auto block_hash{uint256::FromHex(raw_blockhash)}; + if (!block_hash) { return RESTERR(req, HTTP_BAD_REQUEST, "Invalid hash: " + raw_blockhash); } @@ -413,7 +415,7 @@ static bool rest_filter_header(const std::any& context, HTTPRequest* req, const ChainstateManager& chainman = *maybe_chainman; LOCK(cs_main); CChain& active_chain = chainman.ActiveChain(); - const CBlockIndex* pindex = chainman.m_blockman.LookupBlockIndex(block_hash); + const CBlockIndex* pindex{chainman.m_blockman.LookupBlockIndex(*block_hash)}; while (pindex != nullptr && active_chain.Contains(pindex)) { headers.push_back(pindex); if (headers.size() == *parsed_count) @@ -494,8 +496,8 @@ static bool rest_block_filter(const std::any& context, HTTPRequest* req, const s return RESTERR(req, HTTP_BAD_REQUEST, "Invalid URI format. Expected /rest/blockfilter//"); } - uint256 block_hash; - if (!ParseHashStr(uri_parts[1], block_hash)) { + auto block_hash{uint256::FromHex(uri_parts[1])}; + if (!block_hash) { return RESTERR(req, HTTP_BAD_REQUEST, "Invalid hash: " + uri_parts[1]); } @@ -516,7 +518,7 @@ static bool rest_block_filter(const std::any& context, HTTPRequest* req, const s if (!maybe_chainman) return false; ChainstateManager& chainman = *maybe_chainman; LOCK(cs_main); - block_index = chainman.m_blockman.LookupBlockIndex(block_hash); + block_index = chainman.m_blockman.LookupBlockIndex(*block_hash); if (!block_index) { return RESTERR(req, HTTP_NOT_FOUND, uri_parts[1] + " not found"); } @@ -616,14 +618,14 @@ static bool rest_deploymentinfo(const std::any& context, HTTPRequest* req, const jsonRequest.params = UniValue(UniValue::VARR); if (!hash_str.empty()) { - uint256 hash; - if (!ParseHashStr(hash_str, hash)) { + auto hash{uint256::FromHex(hash_str)}; + if (!hash) { return RESTERR(req, HTTP_BAD_REQUEST, "Invalid hash: " + hash_str); } const ChainstateManager* chainman = GetChainman(context, req); if (!chainman) return false; - if (!WITH_LOCK(::cs_main, return chainman->m_blockman.LookupBlockIndex(ParseHashV(hash_str, "blockhash")))) { + if (!WITH_LOCK(::cs_main, return chainman->m_blockman.LookupBlockIndex(*hash))) { return RESTERR(req, HTTP_BAD_REQUEST, "Block not found"); } @@ -704,9 +706,10 @@ static bool rest_tx(const std::any& context, HTTPRequest* req, const std::string std::string hashStr; const RESTResponseFormat rf = ParseDataFormat(hashStr, strURIPart); - uint256 hash; - if (!ParseHashStr(hashStr, hash)) + auto hash{uint256::FromHex(hashStr)}; + if (!hash) { return RESTERR(req, HTTP_BAD_REQUEST, "Invalid hash: " + hashStr); + } if (g_txindex) { g_txindex->BlockUntilSyncedToCurrentChain(); @@ -715,7 +718,7 @@ static bool rest_tx(const std::any& context, HTTPRequest* req, const std::string const NodeContext* const node = GetNodeContext(context, req); if (!node) return false; uint256 hashBlock = uint256(); - const CTransactionRef tx = GetTransaction(/*block_index=*/nullptr, node->mempool.get(), hash, hashBlock, node->chainman->m_blockman); + const CTransactionRef tx{GetTransaction(/*block_index=*/nullptr, node->mempool.get(), *hash, hashBlock, node->chainman->m_blockman)}; if (!tx) { return RESTERR(req, HTTP_NOT_FOUND, hashStr + " not found"); } @@ -792,13 +795,14 @@ static bool rest_getutxos(const std::any& context, HTTPRequest* req, const std:: if (txid_out.size() != 2) { return RESTERR(req, HTTP_BAD_REQUEST, "Parse error"); } + auto txid{Txid::FromHex(txid_out.at(0))}; auto output{ToIntegral(txid_out.at(1))}; - if (!output || !IsHex(txid_out.at(0))) { + if (!txid || !output) { return RESTERR(req, HTTP_BAD_REQUEST, "Parse error"); } - vOutPoints.emplace_back(TxidFromString(txid_out.at(0)), *output); + vOutPoints.emplace_back(*txid, *output); } if (vOutPoints.size() > 0) diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index 8482ce6eb2e..d41ec5bdc62 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -345,13 +345,11 @@ static RPCHelpMan generateblock() std::vector txs; const auto raw_txs_or_txids = request.params[1].get_array(); for (size_t i = 0; i < raw_txs_or_txids.size(); i++) { - const auto str(raw_txs_or_txids[i].get_str()); + const auto& str{raw_txs_or_txids[i].get_str()}; - uint256 hash; CMutableTransaction mtx; - if (ParseHashStr(str, hash)) { - - const auto tx = mempool.get(hash); + if (auto hash{uint256::FromHex(str)}) { + const auto tx{mempool.get(*hash)}; if (!tx) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("Transaction %s not in mempool.", str)); } diff --git a/src/test/blockfilter_tests.cpp b/src/test/blockfilter_tests.cpp index b372f25ea95..470fdde30a3 100644 --- a/src/test/blockfilter_tests.cpp +++ b/src/test/blockfilter_tests.cpp @@ -149,8 +149,7 @@ BOOST_AUTO_TEST_CASE(blockfilters_json_test) unsigned int pos = 0; /*int block_height =*/ test[pos++].getInt(); - uint256 block_hash; - BOOST_CHECK(ParseHashStr(test[pos++].get_str(), block_hash)); + BOOST_CHECK(uint256::FromHex(test[pos++].get_str())); CBlock block; BOOST_REQUIRE(DecodeHexBlk(block, test[pos++].get_str())); @@ -165,11 +164,9 @@ BOOST_AUTO_TEST_CASE(blockfilters_json_test) tx_undo.vprevout.emplace_back(txout, 0, false); } - uint256 prev_filter_header_basic; - BOOST_CHECK(ParseHashStr(test[pos++].get_str(), prev_filter_header_basic)); + uint256 prev_filter_header_basic{*Assert(uint256::FromHex(test[pos++].get_str()))}; std::vector filter_basic = ParseHex(test[pos++].get_str()); - uint256 filter_header_basic; - BOOST_CHECK(ParseHashStr(test[pos++].get_str(), filter_header_basic)); + uint256 filter_header_basic{*Assert(uint256::FromHex(test[pos++].get_str()))}; BlockFilter computed_filter_basic(BlockFilterType::BASIC, block, block_undo); BOOST_CHECK(computed_filter_basic.GetFilter().GetEncoded() == filter_basic); diff --git a/src/test/fuzz/hex.cpp b/src/test/fuzz/hex.cpp index f67b820d11e..bc46863f1df 100644 --- a/src/test/fuzz/hex.cpp +++ b/src/test/fuzz/hex.cpp @@ -27,8 +27,7 @@ FUZZ_TARGET(hex) assert(ToLower(random_hex_string) == hex_data); } (void)IsHexNumber(random_hex_string); - uint256 result; - (void)ParseHashStr(random_hex_string, result); + (void)uint256::FromHex(random_hex_string); (void)uint256S(random_hex_string); try { (void)HexToPubKey(random_hex_string); diff --git a/src/test/pow_tests.cpp b/src/test/pow_tests.cpp index 3a44d1da499..efca5402408 100644 --- a/src/test/pow_tests.cpp +++ b/src/test/pow_tests.cpp @@ -86,7 +86,7 @@ BOOST_AUTO_TEST_CASE(CheckProofOfWork_test_negative_target) uint256 hash; unsigned int nBits; nBits = UintToArith256(consensus.powLimit).GetCompact(true); - hash.SetHex("0x1"); + hash = uint256{1}; BOOST_CHECK(!CheckProofOfWork(hash, nBits, consensus)); } @@ -95,7 +95,7 @@ BOOST_AUTO_TEST_CASE(CheckProofOfWork_test_overflow_target) const auto consensus = CreateChainParams(*m_node.args, ChainType::MAIN)->GetConsensus(); uint256 hash; unsigned int nBits{~0x00800000U}; - hash.SetHex("0x1"); + hash = uint256{1}; BOOST_CHECK(!CheckProofOfWork(hash, nBits, consensus)); } @@ -107,7 +107,7 @@ BOOST_AUTO_TEST_CASE(CheckProofOfWork_test_too_easy_target) arith_uint256 nBits_arith = UintToArith256(consensus.powLimit); nBits_arith *= 2; nBits = nBits_arith.GetCompact(); - hash.SetHex("0x1"); + hash = uint256{1}; BOOST_CHECK(!CheckProofOfWork(hash, nBits, consensus)); } diff --git a/src/test/uint256_tests.cpp b/src/test/uint256_tests.cpp index 669983c0e95..d280126cde4 100644 --- a/src/test/uint256_tests.cpp +++ b/src/test/uint256_tests.cpp @@ -62,7 +62,7 @@ static std::string ArrayToString(const unsigned char A[], unsigned int width) inline uint160 uint160S(std::string_view str) { uint160 rv; - rv.SetHex(str); + rv.SetHexDeprecated(str); return rv; } @@ -157,7 +157,7 @@ BOOST_AUTO_TEST_CASE( comparison ) // <= >= < > uint256S("1000000000000000000000000000000000000000000000000000000000000002")); } -BOOST_AUTO_TEST_CASE( methods ) // GetHex SetHex begin() end() size() GetLow64 GetSerializeSize, Serialize, Unserialize +BOOST_AUTO_TEST_CASE(methods) // GetHex SetHexDeprecated FromHex begin() end() size() GetLow64 GetSerializeSize, Serialize, Unserialize { BOOST_CHECK_EQUAL(R1L.GetHex(), R1L.ToString()); BOOST_CHECK_EQUAL(R2L.GetHex(), R2L.ToString()); @@ -166,12 +166,12 @@ BOOST_AUTO_TEST_CASE( methods ) // GetHex SetHex begin() end() size() GetLow64 G uint256 TmpL(R1L); BOOST_CHECK_EQUAL(TmpL, R1L); // Verify previous values don't persist when setting to truncated string. - TmpL.SetHex("21"); + TmpL.SetHexDeprecated("21"); BOOST_CHECK_EQUAL(TmpL.ToString(), "0000000000000000000000000000000000000000000000000000000000000021"); - TmpL.SetHex(R2L.ToString()); BOOST_CHECK_EQUAL(TmpL, R2L); - TmpL.SetHex(ZeroL.ToString()); BOOST_CHECK_EQUAL(TmpL, uint256()); + BOOST_CHECK_EQUAL(uint256::FromHex(R2L.ToString()).value(), R2L); + BOOST_CHECK_EQUAL(uint256::FromHex(ZeroL.ToString()).value(), uint256()); - TmpL.SetHex(R1L.ToString()); + TmpL = uint256::FromHex(R1L.ToString()).value(); BOOST_CHECK_EQUAL_COLLECTIONS(R1L.begin(), R1L.end(), R1Array, R1Array + R1L.size()); BOOST_CHECK_EQUAL_COLLECTIONS(TmpL.begin(), TmpL.end(), R1Array, R1Array + TmpL.size()); BOOST_CHECK_EQUAL_COLLECTIONS(R2L.begin(), R2L.end(), R2Array, R2Array + R2L.size()); @@ -214,10 +214,10 @@ BOOST_AUTO_TEST_CASE( methods ) // GetHex SetHex begin() end() size() GetLow64 G BOOST_CHECK_EQUAL(MaxS.GetHex(), MaxS.ToString()); uint160 TmpS(R1S); BOOST_CHECK_EQUAL(TmpS, R1S); - TmpS.SetHex(R2S.ToString()); BOOST_CHECK_EQUAL(TmpS, R2S); - TmpS.SetHex(ZeroS.ToString()); BOOST_CHECK_EQUAL(TmpS, uint160()); + BOOST_CHECK_EQUAL(uint160::FromHex(R2S.ToString()).value(), R2S); + BOOST_CHECK_EQUAL(uint160::FromHex(ZeroS.ToString()).value(), uint160()); - TmpS.SetHex(R1S.ToString()); + TmpS = uint160::FromHex(R1S.ToString()).value(); BOOST_CHECK_EQUAL_COLLECTIONS(R1S.begin(), R1S.end(), R1Array, R1Array + R1S.size()); BOOST_CHECK_EQUAL_COLLECTIONS(TmpS.begin(), TmpS.end(), R1Array, R1Array + TmpS.size()); BOOST_CHECK_EQUAL_COLLECTIONS(R2S.begin(), R2S.end(), R2Array, R2Array + R2S.size()); diff --git a/src/uint256.cpp b/src/uint256.cpp index f15150ca956..2756a7f5cd8 100644 --- a/src/uint256.cpp +++ b/src/uint256.cpp @@ -18,7 +18,7 @@ std::string base_blob::GetHex() const } template -void base_blob::SetHex(const std::string_view str) +void base_blob::SetHexDeprecated(const std::string_view str) { std::fill(m_data.begin(), m_data.end(), 0); @@ -52,12 +52,12 @@ std::string base_blob::ToString() const // Explicit instantiations for base_blob<160> template std::string base_blob<160>::GetHex() const; template std::string base_blob<160>::ToString() const; -template void base_blob<160>::SetHex(std::string_view); +template void base_blob<160>::SetHexDeprecated(std::string_view); // Explicit instantiations for base_blob<256> template std::string base_blob<256>::GetHex() const; template std::string base_blob<256>::ToString() const; -template void base_blob<256>::SetHex(std::string_view); +template void base_blob<256>::SetHexDeprecated(std::string_view); const uint256 uint256::ZERO(0); const uint256 uint256::ONE(1); diff --git a/src/uint256.h b/src/uint256.h index 406a9c7203c..f8e78b820f3 100644 --- a/src/uint256.h +++ b/src/uint256.h @@ -1,5 +1,5 @@ // Copyright (c) 2009-2010 Satoshi Nakamoto -// Copyright (c) 2009-2022 The Bitcoin Core developers +// Copyright (c) 2009-present The Bitcoin Core developers // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. @@ -8,12 +8,14 @@ #include #include +#include #include #include #include +#include #include -#include +#include #include /** Template base class for fixed-sized opaque blobs. */ @@ -59,7 +61,8 @@ public: // Hex string representations are little-endian. std::string GetHex() const; - void SetHex(std::string_view str); + /** Unlike FromHex this accepts any invalid input, thus it is fragile and deprecated */ + void SetHexDeprecated(std::string_view str); std::string ToString() const; constexpr const unsigned char* data() const { return m_data.data(); } @@ -88,12 +91,30 @@ public: } }; +namespace detail { +/** + * Writes the hex string (treated as little-endian) into a new uintN_t object + * and only returns a value iff all of the checks pass: + * - Input length is uintN_t::size()*2 + * - All characters are hex + */ +template +std::optional FromHex(std::string_view str) +{ + if (uintN_t::size() * 2 != str.size() || !IsHex(str)) return std::nullopt; + uintN_t rv; + rv.SetHexDeprecated(str); + return rv; +} +} // namespace detail + /** 160-bit opaque blob. * @note This type is called uint160 for historical reasons only. It is an opaque * blob of 160 bits and has no integer operations. */ class uint160 : public base_blob<160> { public: + static std::optional FromHex(std::string_view str) { return detail::FromHex(str); } constexpr uint160() = default; constexpr explicit uint160(Span vch) : base_blob<160>(vch) {} }; @@ -105,6 +126,7 @@ public: */ class uint256 : public base_blob<256> { public: + static std::optional FromHex(std::string_view str) { return detail::FromHex(str); } constexpr uint256() = default; constexpr explicit uint256(uint8_t v) : base_blob<256>(v) {} constexpr explicit uint256(Span vch) : base_blob<256>(vch) {} @@ -113,13 +135,12 @@ public: }; /* uint256 from std::string_view, treated as little-endian. - * This is not a uint256 constructor because of historical fears of uint256(0) - * resolving to a NULL string and crashing. + * DEPRECATED. Unlike FromHex this accepts any invalid input, thus it is fragile and deprecated! */ inline uint256 uint256S(std::string_view str) { uint256 rv; - rv.SetHex(str); + rv.SetHexDeprecated(str); return rv; } diff --git a/src/util/transaction_identifier.h b/src/util/transaction_identifier.h index 1ca48a04547..0e3251f4fce 100644 --- a/src/util/transaction_identifier.h +++ b/src/util/transaction_identifier.h @@ -42,6 +42,12 @@ public: /** Wrapped `uint256` methods. */ constexpr bool IsNull() const { return m_wrapped.IsNull(); } constexpr void SetNull() { m_wrapped.SetNull(); } + static std::optional FromHex(std::string_view hex) + { + auto u{uint256::FromHex(hex)}; + if (!u) return std::nullopt; + return FromUint256(*u); + } std::string GetHex() const { return m_wrapped.GetHex(); } std::string ToString() const { return m_wrapped.ToString(); } static constexpr auto size() { return decltype(m_wrapped)::size(); } @@ -66,6 +72,7 @@ using Txid = transaction_identifier; /** Wtxid commits to all transaction fields including the witness. */ using Wtxid = transaction_identifier; +/** DEPRECATED due to missing length-check and hex-check, please use the safer FromHex, or FromUint256 */ inline Txid TxidFromString(std::string_view str) { return Txid::FromUint256(uint256S(str)); diff --git a/test/functional/interface_rest.py b/test/functional/interface_rest.py index d547da9cf24..ba6e960476b 100755 --- a/test/functional/interface_rest.py +++ b/test/functional/interface_rest.py @@ -208,6 +208,8 @@ class RESTTest (BitcoinTestFramework): self.test_rest_request(f"/getutxos/{spending[0]}_+1", ret_type=RetType.OBJ, status=400) self.test_rest_request(f"/getutxos/{spending[0]}-+1", ret_type=RetType.OBJ, status=400) self.test_rest_request(f"/getutxos/{spending[0]}--1", ret_type=RetType.OBJ, status=400) + self.test_rest_request(f"/getutxos/{spending[0]}aa-1234", ret_type=RetType.OBJ, status=400) + self.test_rest_request(f"/getutxos/aa-1234", ret_type=RetType.OBJ, status=400) # Test limits long_uri = '/'.join([f"{txid}-{n_}" for n_ in range(20)])