Skip to content

Commit 5eb20f8

Browse files
EmpactEmpact
committed
Consistently use ParseHashV to validate hash inputs in rpc
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)"
1 parent 56f6936 commit 5eb20f8

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
@@ -252,7 +252,7 @@ static UniValue waitforblock(const JSONRPCRequest& request)
252252
);
253253
int timeout = 0;
254254

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

257257
if (!request.params[1].isNull())
258258
timeout = request.params[1].get_int();
@@ -706,8 +706,7 @@ static UniValue getblockheader(const JSONRPCRequest& request)
706706

707707
LOCK(cs_main);
708708

709-
std::string strHash = request.params[0].get_str();
710-
uint256 hash(uint256S(strHash));
709+
uint256 hash(ParseHashV(request.params[0], "hash"));
711710

712711
bool fVerbose = true;
713712
if (!request.params[1].isNull())
@@ -800,8 +799,7 @@ static UniValue getblock(const JSONRPCRequest& request)
800799

801800
LOCK(cs_main);
802801

803-
std::string strHash = request.params[0].get_str();
804-
uint256 hash(uint256S(strHash));
802+
uint256 hash(ParseHashV(request.params[0], "blockhash"));
805803

806804
int verbosity = 1;
807805
if (!request.params[1].isNull()) {
@@ -1033,8 +1031,7 @@ UniValue gettxout(const JSONRPCRequest& request)
10331031

10341032
UniValue ret(UniValue::VOBJ);
10351033

1036-
std::string strHash = request.params[0].get_str();
1037-
uint256 hash(uint256S(strHash));
1034+
uint256 hash(ParseHashV(request.params[0], "txid"));
10381035
int n = request.params[1].get_int();
10391036
COutPoint out(hash, n);
10401037
bool fMempool = true;
@@ -1442,8 +1439,7 @@ static UniValue preciousblock(const JSONRPCRequest& request)
14421439
+ HelpExampleRpc("preciousblock", "\"blockhash\"")
14431440
);
14441441

1445-
std::string strHash = request.params[0].get_str();
1446-
uint256 hash(uint256S(strHash));
1442+
uint256 hash(ParseHashV(request.params[0], "blockhash"));
14471443
CBlockIndex* pblockindex;
14481444

14491445
{
@@ -1478,8 +1474,7 @@ static UniValue invalidateblock(const JSONRPCRequest& request)
14781474
+ HelpExampleRpc("invalidateblock", "\"blockhash\"")
14791475
);
14801476

1481-
std::string strHash = request.params[0].get_str();
1482-
uint256 hash(uint256S(strHash));
1477+
uint256 hash(ParseHashV(request.params[0], "blockhash"));
14831478
CValidationState state;
14841479

14851480
{
@@ -1518,8 +1513,7 @@ static UniValue reconsiderblock(const JSONRPCRequest& request)
15181513
+ HelpExampleRpc("reconsiderblock", "\"blockhash\"")
15191514
);
15201515

1521-
std::string strHash = request.params[0].get_str();
1522-
uint256 hash(uint256S(strHash));
1516+
uint256 hash(ParseHashV(request.params[0], "blockhash"));
15231517

15241518
{
15251519
LOCK(cs_main);
@@ -1572,7 +1566,7 @@ static UniValue getchaintxstats(const JSONRPCRequest& request)
15721566
LOCK(cs_main);
15731567
pindex = chainActive.Tip();
15741568
} else {
1575-
uint256 hash = uint256S(request.params[1].get_str());
1569+
uint256 hash(ParseHashV(request.params[1], "blockhash"));
15761570
LOCK(cs_main);
15771571
pindex = LookupBlockIndex(hash);
15781572
if (!pindex) {
@@ -1711,8 +1705,7 @@ static UniValue getblockstats(const JSONRPCRequest& request)
17111705

17121706
pindex = chainActive[height];
17131707
} else {
1714-
const std::string strHash = request.params[0].get_str();
1715-
const uint256 hash(uint256S(strHash));
1708+
const uint256 hash(ParseHashV(request.params[0], "hash_or_height"));
17161709
pindex = LookupBlockIndex(hash);
17171710
if (!pindex) {
17181711
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)) {
@@ -456,7 +456,7 @@ static UniValue getblocktemplate(const JSONRPCRequest& request)
456456
// Format: <hashBestChain><nTransactionsUpdatedLast>
457457
std::string lpstr = lpval.get_str();
458458

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

‎src/rpc/rawtransaction.cpp

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -229,9 +229,7 @@ static UniValue gettxoutproof(const JSONRPCRequest& request)
229229
UniValue txids = request.params[0].get_array();
230230
for (unsigned int idx = 0; idx < txids.size(); idx++) {
231231
const UniValue& txid = txids[idx];
232-
if (txid.get_str().length() != 64 || !IsHex(txid.get_str()))
233-
throw JSONRPCError(RPC_INVALID_PARAMETER, std::string("Invalid txid ")+txid.get_str());
234-
uint256 hash(uint256S(txid.get_str()));
232+
uint256 hash(ParseHashV(txid, "txid"));
235233
if (setTxids.count(hash))
236234
throw JSONRPCError(RPC_INVALID_PARAMETER, std::string("Invalid parameter, duplicated txid: ")+txid.get_str());
237235
setTxids.insert(hash);
@@ -242,7 +240,7 @@ static UniValue gettxoutproof(const JSONRPCRequest& request)
242240
uint256 hashBlock;
243241
if (!request.params[1].isNull()) {
244242
LOCK(cs_main);
245-
hashBlock = uint256S(request.params[1].get_str());
243+
hashBlock = ParseHashV(request.params[1], "blockhash");
246244
pblockindex = LookupBlockIndex(hashBlock);
247245
if (!pblockindex) {
248246
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
@@ -117,16 +117,12 @@ CAmount AmountFromValue(const UniValue& value)
117117

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

‎src/wallet/rpcdump.cpp

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

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

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

‎src/wallet/rpcwallet.cpp

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

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

2236-
blockId.SetHex(request.params[0].get_str());
22372236
paltindex = pindex = LookupBlockIndex(blockId);
22382237
if (!pindex) {
22392238
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Block not found");
@@ -2362,8 +2361,7 @@ static UniValue gettransaction(const JSONRPCRequest& request)
23622361

23632362
LOCK2(cs_main, pwallet->cs_wallet);
23642363

2365-
uint256 hash;
2366-
hash.SetHex(request.params[0].get_str());
2364+
uint256 hash(ParseHashV(request.params[0], "txid"));
23672365

23682366
isminefilter filter = ISMINE_SPENDABLE;
23692367
if(!request.params[1].isNull())
@@ -2430,8 +2428,7 @@ static UniValue abandontransaction(const JSONRPCRequest& request)
24302428

24312429
LOCK2(cs_main, pwallet->cs_wallet);
24322430

2433-
uint256 hash;
2434-
hash.SetHex(request.params[0].get_str());
2431+
uint256 hash(ParseHashV(request.params[0], "txid"));
24352432

24362433
if (!pwallet->mapWallet.count(hash)) {
24372434
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid or non-wallet transaction id");
@@ -2836,17 +2833,13 @@ static UniValue lockunspent(const JSONRPCRequest& request)
28362833
{"vout", UniValueType(UniValue::VNUM)},
28372834
});
28382835

2839-
const std::string& txid = find_value(o, "txid").get_str();
2840-
if (!IsHex(txid)) {
2841-
throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, expected hex txid");
2842-
}
2843-
2836+
const uint256 txid(ParseHashO(o, "txid"));
28442837
const int nOutput = find_value(o, "vout").get_int();
28452838
if (nOutput < 0) {
28462839
throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, vout must be positive");
28472840
}
28482841

2849-
const COutPoint outpt(uint256S(txid), nOutput);
2842+
const COutPoint outpt(txid, nOutput);
28502843

28512844
const auto it = pwallet->mapWallet.find(outpt.hash);
28522845
if (it == pwallet->mapWallet.end()) {
@@ -3701,8 +3694,7 @@ static UniValue bumpfee(const JSONRPCRequest& request)
37013694
}
37023695

37033696
RPCTypeCheck(request.params, {UniValue::VSTR, UniValue::VOBJ});
3704-
uint256 hash;
3705-
hash.SetHex(request.params[0].get_str());
3697+
uint256 hash(ParseHashV(request.params[0], "txid"));
37063698

37073699
// optional parameters
37083700
CAmount totalFee = 0;

‎test/functional/mining_prioritisetransaction.py

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

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

2930
# Test `prioritisetransaction` invalid `dummy`
3031
txid = '1d1d4e24ed99057e84c3f80fd8fbec79ed9e1acee37da269356ecea000000000'

‎test/functional/p2p_compactblocks.py

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

289289
# Store the raw block in our internal format.
290-
block = FromHex(CBlock(), node.getblock("%02x" % block_hash, False))
290+
block = FromHex(CBlock(), node.getblock("%064x" % block_hash, False))
291291
for tx in block.vtx:
292292
tx.calc_sha256()
293293
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)
@@ -206,7 +208,9 @@ def _test_gettxoutsetinfo(self):
206208
def _test_getblockheader(self):
207209
node = self.nodes[0]
208210

209-
assert_raises_rpc_error(-5, "Block not found", node.getblockheader, "nonsense")
211+
assert_raises_rpc_error(-8, "hash must be of length 64 (not 8, for 'nonsense')", node.getblockheader, "nonsense")
212+
assert_raises_rpc_error(-8, "hash must be hexadecimal string (not 'ZZZ7bb8b1697ea987f3b223ba7819250cae33efacb068d23dc24859824a77844')", node.getblockheader, "ZZZ7bb8b1697ea987f3b223ba7819250cae33efacb068d23dc24859824a77844")
213+
assert_raises_rpc_error(-5, "Block not found", node.getblockheader, "0cf7bb8b1697ea987f3b223ba7819250cae33efacb068d23dc24859824a77844")
210214

211215
besthash = node.getbestblockhash()
212216
secondbesthash = node.getblockhash(199)

0 commit comments

Comments
 (0)