Skip to content

Commit 3761209

Browse files
author
MarcoFalke
committed
Merge #13424: Consistently validate txid / blockhash length and encoding in rpc calls
5eb20f8 Consistently use ParseHashV to validate hash inputs in rpc (Ben Woosley) Pull request description: ParseHashV validates the length and encoding of the string and throws an informative RPC error on failure, which is as good or better than these alternative calls. Note I switched ParseHashV to check string length first, because IsHex tests that the length is even, and an error like: "must be of length 64 (not 63, for X)" is much more informative than "must be hexadecimal string (not X)" in that case. Split from #13420 Tree-SHA512: f0786b41c0d7793ff76e4b2bb35547873070bbf7561d510029e8edb93f59176277efcd4d183b3185532ea69fc0bbbf3dbe9e19362e8017007ae9d51266cd78ae
2 parents 985d28c + 5eb20f8 commit 3761209

14 files changed

+58
-59
lines changed

‎src/rpc/blockchain.cpp

Lines changed: 9 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -260,7 +260,7 @@ static UniValue waitforblock(const JSONRPCRequest& request)
260260
);
261261
int timeout = 0;
262262

263-
uint256 hash = uint256S(request.params[0].get_str());
263+
uint256 hash(ParseHashV(request.params[0], "blockhash"));
264264

265265
if (!request.params[1].isNull())
266266
timeout = request.params[1].get_int();
@@ -727,8 +727,7 @@ static UniValue getblockheader(const JSONRPCRequest& request)
727727

728728
LOCK(cs_main);
729729

730-
std::string strHash = request.params[0].get_str();
731-
uint256 hash(uint256S(strHash));
730+
uint256 hash(ParseHashV(request.params[0], "hash"));
732731

733732
bool fVerbose = true;
734733
if (!request.params[1].isNull())
@@ -822,8 +821,7 @@ static UniValue getblock(const JSONRPCRequest& request)
822821

823822
LOCK(cs_main);
824823

825-
std::string strHash = request.params[0].get_str();
826-
uint256 hash(uint256S(strHash));
824+
uint256 hash(ParseHashV(request.params[0], "blockhash"));
827825

828826
int verbosity = 1;
829827
if (!request.params[1].isNull()) {
@@ -1055,8 +1053,7 @@ UniValue gettxout(const JSONRPCRequest& request)
10551053

10561054
UniValue ret(UniValue::VOBJ);
10571055

1058-
std::string strHash = request.params[0].get_str();
1059-
uint256 hash(uint256S(strHash));
1056+
uint256 hash(ParseHashV(request.params[0], "txid"));
10601057
int n = request.params[1].get_int();
10611058
COutPoint out(hash, n);
10621059
bool fMempool = true;
@@ -1464,8 +1461,7 @@ static UniValue preciousblock(const JSONRPCRequest& request)
14641461
+ HelpExampleRpc("preciousblock", "\"blockhash\"")
14651462
);
14661463

1467-
std::string strHash = request.params[0].get_str();
1468-
uint256 hash(uint256S(strHash));
1464+
uint256 hash(ParseHashV(request.params[0], "blockhash"));
14691465
CBlockIndex* pblockindex;
14701466

14711467
{
@@ -1500,8 +1496,7 @@ static UniValue invalidateblock(const JSONRPCRequest& request)
15001496
+ HelpExampleRpc("invalidateblock", "\"blockhash\"")
15011497
);
15021498

1503-
std::string strHash = request.params[0].get_str();
1504-
uint256 hash(uint256S(strHash));
1499+
uint256 hash(ParseHashV(request.params[0], "blockhash"));
15051500
CValidationState state;
15061501

15071502
{
@@ -1540,8 +1535,7 @@ static UniValue reconsiderblock(const JSONRPCRequest& request)
15401535
+ HelpExampleRpc("reconsiderblock", "\"blockhash\"")
15411536
);
15421537

1543-
std::string strHash = request.params[0].get_str();
1544-
uint256 hash(uint256S(strHash));
1538+
uint256 hash(ParseHashV(request.params[0], "blockhash"));
15451539

15461540
{
15471541
LOCK(cs_main);
@@ -1594,7 +1588,7 @@ static UniValue getchaintxstats(const JSONRPCRequest& request)
15941588
LOCK(cs_main);
15951589
pindex = chainActive.Tip();
15961590
} else {
1597-
uint256 hash = uint256S(request.params[1].get_str());
1591+
uint256 hash(ParseHashV(request.params[1], "blockhash"));
15981592
LOCK(cs_main);
15991593
pindex = LookupBlockIndex(hash);
16001594
if (!pindex) {
@@ -1768,8 +1762,7 @@ static UniValue getblockstats(const JSONRPCRequest& request)
17681762

17691763
pindex = chainActive[height];
17701764
} else {
1771-
const std::string strHash = request.params[0].get_str();
1772-
const uint256 hash(uint256S(strHash));
1765+
const uint256 hash(ParseHashV(request.params[0], "hash_or_height"));
17731766
pindex = LookupBlockIndex(hash);
17741767
if (!pindex) {
17751768
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Block not found");

‎src/rpc/mining.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -247,7 +247,7 @@ static UniValue prioritisetransaction(const JSONRPCRequest& request)
247247

248248
LOCK(cs_main);
249249

250-
uint256 hash = ParseHashStr(request.params[0].get_str(), "txid");
250+
uint256 hash(ParseHashV(request.params[0], "txid"));
251251
CAmount nAmount = request.params[2].get_int64();
252252

253253
if (!(request.params[1].isNull() || request.params[1].get_real() == 0)) {
@@ -455,7 +455,7 @@ static UniValue getblocktemplate(const JSONRPCRequest& request)
455455
// Format: <hashBestChain><nTransactionsUpdatedLast>
456456
std::string lpstr = lpval.get_str();
457457

458-
hashWatchedChain.SetHex(lpstr.substr(0, 64));
458+
hashWatchedChain = ParseHashV(lpstr.substr(0, 64), "longpollid");
459459
nTransactionsUpdatedLastLP = atoi64(lpstr.substr(64));
460460
}
461461
else

‎src/rpc/rawtransaction.cpp

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -226,9 +226,7 @@ static UniValue gettxoutproof(const JSONRPCRequest& request)
226226
UniValue txids = request.params[0].get_array();
227227
for (unsigned int idx = 0; idx < txids.size(); idx++) {
228228
const UniValue& txid = txids[idx];
229-
if (txid.get_str().length() != 64 || !IsHex(txid.get_str()))
230-
throw JSONRPCError(RPC_INVALID_PARAMETER, std::string("Invalid txid ")+txid.get_str());
231-
uint256 hash(uint256S(txid.get_str()));
229+
uint256 hash(ParseHashV(txid, "txid"));
232230
if (setTxids.count(hash))
233231
throw JSONRPCError(RPC_INVALID_PARAMETER, std::string("Invalid parameter, duplicated txid: ")+txid.get_str());
234232
setTxids.insert(hash);
@@ -239,7 +237,7 @@ static UniValue gettxoutproof(const JSONRPCRequest& request)
239237
uint256 hashBlock;
240238
if (!request.params[1].isNull()) {
241239
LOCK(cs_main);
242-
hashBlock = uint256S(request.params[1].get_str());
240+
hashBlock = ParseHashV(request.params[1], "blockhash");
243241
pblockindex = LookupBlockIndex(hashBlock);
244242
if (!pblockindex) {
245243
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Block not found");

‎src/rpc/server.cpp

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -116,16 +116,12 @@ CAmount AmountFromValue(const UniValue& value)
116116

117117
uint256 ParseHashV(const UniValue& v, std::string strName)
118118
{
119-
std::string strHex;
120-
if (v.isStr())
121-
strHex = v.get_str();
119+
std::string strHex(v.get_str());
120+
if (64 != strHex.length())
121+
throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("%s must be of length %d (not %d, for '%s')", strName, 64, strHex.length(), strHex));
122122
if (!IsHex(strHex)) // Note: IsHex("") is false
123123
throw JSONRPCError(RPC_INVALID_PARAMETER, strName+" must be hexadecimal string (not '"+strHex+"')");
124-
if (64 != strHex.length())
125-
throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("%s must be of length %d (not %d)", strName, 64, strHex.length()));
126-
uint256 result;
127-
result.SetHex(strHex);
128-
return result;
124+
return uint256S(strHex);
129125
}
130126
uint256 ParseHashO(const UniValue& o, std::string strKey)
131127
{

‎src/wallet/rpcdump.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -415,8 +415,7 @@ UniValue removeprunedfunds(const JSONRPCRequest& request)
415415

416416
LOCK2(cs_main, pwallet->cs_wallet);
417417

418-
uint256 hash;
419-
hash.SetHex(request.params[0].get_str());
418+
uint256 hash(ParseHashV(request.params[0], "txid"));
420419
std::vector<uint256> vHash;
421420
vHash.push_back(hash);
422421
std::vector<uint256> vHashOut;

‎src/wallet/rpcwallet.cpp

Lines changed: 6 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1638,9 +1638,8 @@ static UniValue listsinceblock(const JSONRPCRequest& request)
16381638
isminefilter filter = ISMINE_SPENDABLE;
16391639

16401640
if (!request.params[0].isNull() && !request.params[0].get_str().empty()) {
1641-
uint256 blockId;
1641+
uint256 blockId(ParseHashV(request.params[0], "blockhash"));
16421642

1643-
blockId.SetHex(request.params[0].get_str());
16441643
paltindex = pindex = LookupBlockIndex(blockId);
16451644
if (!pindex) {
16461645
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Block not found");
@@ -1768,8 +1767,7 @@ static UniValue gettransaction(const JSONRPCRequest& request)
17681767

17691768
LOCK2(cs_main, pwallet->cs_wallet);
17701769

1771-
uint256 hash;
1772-
hash.SetHex(request.params[0].get_str());
1770+
uint256 hash(ParseHashV(request.params[0], "txid"));
17731771

17741772
isminefilter filter = ISMINE_SPENDABLE;
17751773
if(!request.params[1].isNull())
@@ -1836,8 +1834,7 @@ static UniValue abandontransaction(const JSONRPCRequest& request)
18361834

18371835
LOCK2(cs_main, pwallet->cs_wallet);
18381836

1839-
uint256 hash;
1840-
hash.SetHex(request.params[0].get_str());
1837+
uint256 hash(ParseHashV(request.params[0], "txid"));
18411838

18421839
if (!pwallet->mapWallet.count(hash)) {
18431840
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid or non-wallet transaction id");
@@ -2241,17 +2238,13 @@ static UniValue lockunspent(const JSONRPCRequest& request)
22412238
{"vout", UniValueType(UniValue::VNUM)},
22422239
});
22432240

2244-
const std::string& txid = find_value(o, "txid").get_str();
2245-
if (!IsHex(txid)) {
2246-
throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, expected hex txid");
2247-
}
2248-
2241+
const uint256 txid(ParseHashO(o, "txid"));
22492242
const int nOutput = find_value(o, "vout").get_int();
22502243
if (nOutput < 0) {
22512244
throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, vout must be positive");
22522245
}
22532246

2254-
const COutPoint outpt(uint256S(txid), nOutput);
2247+
const COutPoint outpt(txid, nOutput);
22552248

22562249
const auto it = pwallet->mapWallet.find(outpt.hash);
22572250
if (it == pwallet->mapWallet.end()) {
@@ -3176,8 +3169,7 @@ static UniValue bumpfee(const JSONRPCRequest& request)
31763169
}
31773170

31783171
RPCTypeCheck(request.params, {UniValue::VSTR, UniValue::VOBJ});
3179-
uint256 hash;
3180-
hash.SetHex(request.params[0].get_str());
3172+
uint256 hash(ParseHashV(request.params[0], "txid"));
31813173

31823174
// optional parameters
31833175
CAmount totalFee = 0;

‎test/functional/mining_prioritisetransaction.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,8 @@ def run_test(self):
2929
assert_raises_rpc_error(-1, "prioritisetransaction", self.nodes[0].prioritisetransaction, '', 0, 0, 0)
3030

3131
# Test `prioritisetransaction` invalid `txid`
32-
assert_raises_rpc_error(-1, "txid must be hexadecimal string", self.nodes[0].prioritisetransaction, txid='foo', fee_delta=0)
32+
assert_raises_rpc_error(-8, "txid must be of length 64 (not 3, for 'foo')", self.nodes[0].prioritisetransaction, txid='foo', fee_delta=0)
33+
assert_raises_rpc_error(-8, "txid must be hexadecimal string (not 'Zd1d4e24ed99057e84c3f80fd8fbec79ed9e1acee37da269356ecea000000000')", self.nodes[0].prioritisetransaction, txid='Zd1d4e24ed99057e84c3f80fd8fbec79ed9e1acee37da269356ecea000000000', fee_delta=0)
3334

3435
# Test `prioritisetransaction` invalid `dummy`
3536
txid = '1d1d4e24ed99057e84c3f80fd8fbec79ed9e1acee37da269356ecea000000000'

‎test/functional/p2p_compactblocks.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -293,7 +293,7 @@ def test_compactblock_construction(self, node, test_node, version, use_witness_a
293293
block_hash = int(node.generate(1)[0], 16)
294294

295295
# Store the raw block in our internal format.
296-
block = FromHex(CBlock(), node.getblock("%02x" % block_hash, False))
296+
block = FromHex(CBlock(), node.getblock("%064x" % block_hash, False))
297297
for tx in block.vtx:
298298
tx.calc_sha256()
299299
block.rehash()

‎test/functional/p2p_sendheaders.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -402,7 +402,7 @@ def test_nonnull_locators(self, test_node, inv_node):
402402

403403
block_time += 9
404404

405-
fork_point = self.nodes[0].getblock("%02x" % new_block_hashes[0])["previousblockhash"]
405+
fork_point = self.nodes[0].getblock("%064x" % new_block_hashes[0])["previousblockhash"]
406406
fork_point = int(fork_point, 16)
407407

408408
# Use getblocks/getdata

‎test/functional/rpc_blockchain.py

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,9 @@ def _test_getchaintxstats(self):
125125

126126
# Test `getchaintxstats` invalid `blockhash`
127127
assert_raises_rpc_error(-1, "JSON value is not a string as expected", self.nodes[0].getchaintxstats, blockhash=0)
128-
assert_raises_rpc_error(-5, "Block not found", self.nodes[0].getchaintxstats, blockhash='0')
128+
assert_raises_rpc_error(-8, "blockhash must be of length 64 (not 1, for '0')", self.nodes[0].getchaintxstats, blockhash='0')
129+
assert_raises_rpc_error(-8, "blockhash must be hexadecimal string (not 'ZZZ0000000000000000000000000000000000000000000000000000000000000')", self.nodes[0].getchaintxstats, blockhash='ZZZ0000000000000000000000000000000000000000000000000000000000000')
130+
assert_raises_rpc_error(-5, "Block not found", self.nodes[0].getchaintxstats, blockhash='0000000000000000000000000000000000000000000000000000000000000000')
129131
blockhash = self.nodes[0].getblockhash(200)
130132
self.nodes[0].invalidateblock(blockhash)
131133
assert_raises_rpc_error(-8, "Block is not in main chain", self.nodes[0].getchaintxstats, blockhash=blockhash)
@@ -203,7 +205,9 @@ def _test_gettxoutsetinfo(self):
203205
def _test_getblockheader(self):
204206
node = self.nodes[0]
205207

206-
assert_raises_rpc_error(-5, "Block not found", node.getblockheader, "nonsense")
208+
assert_raises_rpc_error(-8, "hash must be of length 64 (not 8, for 'nonsense')", node.getblockheader, "nonsense")
209+
assert_raises_rpc_error(-8, "hash must be hexadecimal string (not 'ZZZ7bb8b1697ea987f3b223ba7819250cae33efacb068d23dc24859824a77844')", node.getblockheader, "ZZZ7bb8b1697ea987f3b223ba7819250cae33efacb068d23dc24859824a77844")
210+
assert_raises_rpc_error(-5, "Block not found", node.getblockheader, "0cf7bb8b1697ea987f3b223ba7819250cae33efacb068d23dc24859824a77844")
207211

208212
besthash = node.getbestblockhash()
209213
secondbesthash = node.getblockhash(199)

0 commit comments

Comments
 (0)