Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RPC] Consistently use ParseHashV to validate hash inputs in rpc #2734

Merged
merged 1 commit into from
Feb 5, 2022
Merged

[RPC] Consistently use ParseHashV to validate hash inputs in rpc #2734

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 8 additions & 13 deletions src/rpc/blockchain.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -314,7 +314,7 @@ UniValue waitforblock(const JSONRPCRequest& request)
);
int timeout = 0;

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

if (request.params.size() > 1)
timeout = request.params[1].get_int();
Expand Down Expand Up @@ -569,8 +569,7 @@ UniValue getblock(const JSONRPCRequest& request)

LOCK(cs_main);

std::string strHash = request.params[0].get_str();
uint256 hash(uint256S(strHash));
uint256 hash(ParseHashV(request.params[0], "blockhash"));

bool fVerbose = true;
if (request.params.size() > 1)
Expand Down Expand Up @@ -630,15 +629,15 @@ UniValue getblockheader(const JSONRPCRequest& request)
HelpExampleCli("getblockheader", "\"00000000000fd08c2fb661d2fcb0d49abb3a91e5f27082ce64feed3b4dede2e2\"") +
HelpExampleRpc("getblockheader", "\"00000000000fd08c2fb661d2fcb0d49abb3a91e5f27082ce64feed3b4dede2e2\""));

LOCK(cs_main);

std::string strHash = request.params[0].get_str();
uint256 hash(uint256S(strHash));
uint256 hash(ParseHashV(request.params[0], "blockhash"));

bool fVerbose = true;
if (request.params.size() > 1)
fVerbose = request.params[1].get_bool();

LOCK(cs_main);

CBlockIndex* pblockindex = LookupBlockIndex(hash);
if (pblockindex == nullptr)
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Block not found");
Expand Down Expand Up @@ -845,9 +844,7 @@ UniValue gettxout(const JSONRPCRequest& request)
LOCK(cs_main);

UniValue ret(UniValue::VOBJ);

std::string strHash = request.params[0].get_str();
uint256 hash(uint256S(strHash));
uint256 hash(ParseHashV(request.params[0], "txid"));
int n = request.params[1].get_int();
COutPoint out(hash, n);
bool fMempool = true;
Expand Down Expand Up @@ -1207,8 +1204,7 @@ UniValue invalidateblock(const JSONRPCRequest& request)
"\nExamples:\n" +
HelpExampleCli("invalidateblock", "\"blockhash\"") + HelpExampleRpc("invalidateblock", "\"blockhash\""));

std::string strHash = request.params[0].get_str();
uint256 hash(uint256S(strHash));
uint256 hash(ParseHashV(request.params[0], "blockhash"));
CValidationState state;

{
Expand Down Expand Up @@ -1248,8 +1244,7 @@ UniValue reconsiderblock(const JSONRPCRequest& request)
"\nExamples:\n" +
HelpExampleCli("reconsiderblock", "\"blockhash\"") + HelpExampleRpc("reconsiderblock", "\"blockhash\""));

std::string strHash = request.params[0].get_str();
uint256 hash(uint256S(strHash));
uint256 hash(ParseHashV(request.params[0], "blockhash"));
CValidationState state;

{
Expand Down
4 changes: 2 additions & 2 deletions src/rpc/budget.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -700,9 +700,9 @@ UniValue mnfinalbudget(const JSONRPCRequest& request)
if (request.params.size() != 2)
throw std::runtime_error("Correct usage is 'mnbudget getvotes budget-hash'");

uint256 hash(ParseHashV(request.params[1], "budget-hash"));

LOCK(g_budgetman.cs_budgets);
std::string strHash = request.params[1].get_str();
uint256 hash(uint256S(strHash));
CFinalizedBudget* pfinalBudget = g_budgetman.FindFinalizedBudget(hash);
if (pfinalBudget == NULL) return "Unknown budget hash";
return pfinalBudget->GetVotesObject();
Expand Down
2 changes: 1 addition & 1 deletion src/rpc/mining.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -591,7 +591,7 @@ UniValue getblocktemplate(const JSONRPCRequest& request)
// Format: <hashBestChain><nTransactionsUpdatedLast>
std::string lpstr = lpval.get_str();

hashWatchedChain.SetHex(lpstr.substr(0, 64));
hashWatchedChain = ParseHashV(lpstr.substr(0, 64), "longpollid");
nTransactionsUpdatedLastLP = atoi64(lpstr.substr(64));
} else {
// NOTE: Spec does not specify behaviour for non-string longpollid, but this makes testing easier
Expand Down
5 changes: 2 additions & 3 deletions src/rpc/misc.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -791,8 +791,7 @@ UniValue mnconnect(const JSONRPCRequest& request)

uint256 quorum_hash;
if (request.params.size() > 3) {
RPCTypeCheckArgument(request.params[3], UniValue::VSTR);
quorum_hash = uint256S(request.params[3].get_str());
quorum_hash = ParseHashV(request.params[3], "quorum_hash");
}

// First obtain the connection type
Expand All @@ -801,7 +800,7 @@ UniValue mnconnect(const JSONRPCRequest& request)
const auto& array{request.params[1].get_array()};
std::set<uint256> set_dmn_protxhash;
for (unsigned int i = 0; i < array.size(); i++) {
set_dmn_protxhash.emplace(uint256S(array[i].get_str()));
set_dmn_protxhash.emplace(ParseHashV(array[i], strprintf("pro_tx_hash (index %d)", i)));
}

const auto& mn_connan = g_connman->GetTierTwoConnMan();
Expand Down
12 changes: 4 additions & 8 deletions src/rpc/server.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,16 +135,12 @@ UniValue ValueFromAmount(const CAmount& amount)

uint256 ParseHashV(const UniValue& v, std::string strName)
{
std::string strHex;
if (v.isStr())
strHex = v.get_str();
std::string strHex(v.get_str());
if (64 != strHex.length())
throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("%s must be of length %d (not %d, for '%s')", strName, 64, strHex.length(), strHex));
if (!IsHex(strHex)) // Note: IsHex("") is false
throw JSONRPCError(RPC_INVALID_PARAMETER, strName + " must be hexadecimal string (not '" + strHex + "')");
if (64 != strHex.length())
throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("%s must be of length %d (not %d)", strName, 64, strHex.length()));
uint256 result;
result.SetHex(strHex);
return result;
return uint256S(strHex);
}
uint256 ParseHashO(const UniValue& o, std::string strKey)
{
Expand Down
20 changes: 6 additions & 14 deletions src/wallet/rpcwallet.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1530,8 +1530,7 @@ UniValue viewshieldtransaction(const JSONRPCRequest& request)

LOCK2(cs_main, pwallet->cs_wallet);

uint256 hash;
hash.SetHex(request.params[0].get_str());
uint256 hash(ParseHashV(request.params[0], "txid"));

UniValue entry(UniValue::VOBJ);
auto it = pwallet->mapWallet.find(hash);
Expand Down Expand Up @@ -3202,9 +3201,7 @@ UniValue listsinceblock(const JSONRPCRequest& request)
isminefilter filter = ISMINE_SPENDABLE_ALL | ISMINE_COLD;

if (request.params.size() > 0) {
uint256 blockId;

blockId.SetHex(request.params[0].get_str());
uint256 blockId(ParseHashV(request.params[0], "blockhash"));
pindex = LookupBlockIndex(blockId);
}

Expand Down Expand Up @@ -3291,8 +3288,7 @@ UniValue gettransaction(const JSONRPCRequest& request)

LOCK2(cs_main, pwallet->cs_wallet);

uint256 hash;
hash.SetHex(request.params[0].get_str());
uint256 hash(ParseHashV(request.params[0], "txid"));

isminefilter filter = ISMINE_SPENDABLE_ALL | ISMINE_COLD;
if (request.params.size() > 1)
Expand Down Expand Up @@ -3358,8 +3354,7 @@ UniValue abandontransaction(const JSONRPCRequest& request)

LOCK2(cs_main, pwallet->cs_wallet);

uint256 hash;
hash.SetHex(request.params[0].get_str());
uint256 hash(ParseHashV(request.params[0], "txid"));

if (!pwallet->mapWallet.count(hash))
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid or non-wallet transaction id");
Expand Down Expand Up @@ -4085,17 +4080,14 @@ UniValue lockunspent(const JSONRPCRequest& request)
{"vout", UniValueType(UniValue::VNUM)},
});

const std::string& txid = find_value(o, "txid").get_str();
if (!IsHex(txid)) {
throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, expected hex txid");
}
const uint256 txid(ParseHashO(o, "txid"));

const int nOutput = find_value(o, "vout").get_int();
if (nOutput < 0) {
throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, vout must be positive");
}

const COutPoint outpt(uint256S(txid), nOutput);
const COutPoint outpt(txid, nOutput);

const auto it = pwallet->mapWallet.find(outpt.hash);
if (it == pwallet->mapWallet.end()) {
Expand Down
4 changes: 2 additions & 2 deletions test/functional/rpc_blockchain.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,8 @@ def _test_gettxoutsetinfo(self):
def _test_getblockheader(self):
node = self.nodes[0]

assert_raises_rpc_error(-5, "Block not found",
node.getblockheader, "nonsense")
assert_raises_rpc_error(-8, "blockhash must be of length 64 (not 8, for 'nonsense')",
node.getblockheader, "nonsense")

besthash = node.getbestblockhash()
secondbesthash = node.getblockhash(199)
Expand Down
12 changes: 7 additions & 5 deletions test/functional/rpc_rawtransaction.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,9 @@ def run_test(self):
txid = '1d1d4e24ed99057e84c3f80fd8fbec79ed9e1acee37da269356ecea000000000'
assert_raises_rpc_error(-3, "Expected type array", self.nodes[0].createrawtransaction, 'foo', {})
assert_raises_rpc_error(-1, "JSON value is not an object as expected", self.nodes[0].createrawtransaction, ['foo'], {})
assert_raises_rpc_error(-8, "txid must be hexadecimal string", self.nodes[0].createrawtransaction, [{}], {})
assert_raises_rpc_error(-8, "txid must be hexadecimal string", self.nodes[0].createrawtransaction, [{'txid': 'foo'}], {})
assert_raises_rpc_error(-1, "JSON value is not a string as expected", self.nodes[0].createrawtransaction, [{}], {})
assert_raises_rpc_error(-8, "txid must be of length 64 (not 3, for 'foo')", self.nodes[0].createrawtransaction, [{'txid': 'foo'}], {})
assert_raises_rpc_error(-8, "txid must be hexadecimal string (not 'ZZZ7bb8b1697ea987f3b223ba7819250cae33efacb068d23dc24859824a77844')", self.nodes[0].createrawtransaction, [{'txid': 'ZZZ7bb8b1697ea987f3b223ba7819250cae33efacb068d23dc24859824a77844'}], {})
assert_raises_rpc_error(-8, "Invalid parameter, missing vout key", self.nodes[0].createrawtransaction, [{'txid': txid}], {})
assert_raises_rpc_error(-8, "Invalid parameter, missing vout key", self.nodes[0].createrawtransaction, [{'txid': txid, 'vout': 'foo'}], {})
assert_raises_rpc_error(-8, "Invalid parameter, vout must be positive", self.nodes[0].createrawtransaction, [{'txid': txid, 'vout': -1}], {})
Expand Down Expand Up @@ -164,9 +165,10 @@ def run_test(self):
# We should not get the tx if we provide an unrelated block
assert_raises_rpc_error(-5, "No such transaction found", self.nodes[0].getrawtransaction, tx, True, block2)
# An invalid block hash should raise the correct errors
assert_raises_rpc_error(-8, "parameter 3 must be hexadecimal", self.nodes[0].getrawtransaction, tx, True, True)
assert_raises_rpc_error(-8, "parameter 3 must be hexadecimal", self.nodes[0].getrawtransaction, tx, True, "foobar")
assert_raises_rpc_error(-8, "parameter 3 must be of length 64", self.nodes[0].getrawtransaction, tx, True, "abcd1234")
assert_raises_rpc_error(-1, "JSON value is not a string as expected", self.nodes[0].getrawtransaction, tx, True, True)
assert_raises_rpc_error(-8, "parameter 3 must be of length 64 (not 6, for 'foobar')", self.nodes[0].getrawtransaction, tx, True, "foobar")
assert_raises_rpc_error(-8, "parameter 3 must be of length 64 (not 8, for 'abcd1234')", self.nodes[0].getrawtransaction, tx, True, "abcd1234")
assert_raises_rpc_error(-8, "parameter 3 must be hexadecimal string (not 'ZZZ0000000000000000000000000000000000000000000000000000000000000')", self.nodes[0].getrawtransaction, tx, True, "ZZZ0000000000000000000000000000000000000000000000000000000000000")
assert_raises_rpc_error(-5, "Block hash not found", self.nodes[0].getrawtransaction, tx, True, "0000000000000000000000000000000000000000000000000000000000000000")
# Undo the blocks and check in_active_chain
self.nodes[0].invalidateblock(block1)
Expand Down