LBRYCommunity/lbrycrd
1
0
Fork 0
Code Issues 29 Pull requests 1 Projects Releases 32 Packages Wiki Activity

Merge #13424: Consistently validate txid / blockhash length and encoding in rpc calls

Browse source 
5eb20f81d9 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
This commit is contained in:
MarcoFalke 2018-09-24 14:54:10 -04:00
commit 37612099ec
No known key found for this signature in database
GPG key ID: D2EA4850E7528B25
14 changed files with 58 additions and 59 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

25
src/rpc/blockchain.cpp
View file

@ -260,7 +260,7 @@ static 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[1].isNull())
timeout = request.params[1].get_int();
@ -727,8 +727,7 @@ static UniValue getblockheader(const JSONRPCRequest& request)
LOCK(cs_main);
std::string strHash = request.params[0].get_str();
uint256 hash(uint256S(strHash));
uint256 hash(ParseHashV(request.params[0], "hash"));
bool fVerbose = true;
if (!request.params[1].isNull())
@ -822,8 +821,7 @@ static 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"));
int verbosity = 1;
if (!request.params[1].isNull()) {
@ -1055,8 +1053,7 @@ UniValue gettxout(const JSONRPCRequest& request)
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;
@ -1464,8 +1461,7 @@ static UniValue preciousblock(const JSONRPCRequest& request)
+ HelpExampleRpc("preciousblock", "\"blockhash\"")
);
std::string strHash = request.params[0].get_str();
uint256 hash(uint256S(strHash));
uint256 hash(ParseHashV(request.params[0], "blockhash"));
CBlockIndex* pblockindex;
{
@ -1500,8 +1496,7 @@ static UniValue invalidateblock(const JSONRPCRequest& request)
+ HelpExampleRpc("invalidateblock", "\"blockhash\"")
);
std::string strHash = request.params[0].get_str();
uint256 hash(uint256S(strHash));
uint256 hash(ParseHashV(request.params[0], "blockhash"));
CValidationState state;
{
@ -1540,8 +1535,7 @@ static UniValue reconsiderblock(const JSONRPCRequest& request)
+ HelpExampleRpc("reconsiderblock", "\"blockhash\"")
);
std::string strHash = request.params[0].get_str();
uint256 hash(uint256S(strHash));
uint256 hash(ParseHashV(request.params[0], "blockhash"));
{
LOCK(cs_main);
@ -1594,7 +1588,7 @@ static UniValue getchaintxstats(const JSONRPCRequest& request)
LOCK(cs_main);
pindex = chainActive.Tip();
} else {
uint256 hash = uint256S(request.params[1].get_str());
uint256 hash(ParseHashV(request.params[1], "blockhash"));
LOCK(cs_main);
pindex = LookupBlockIndex(hash);
if (!pindex) {
@ -1768,8 +1762,7 @@ static UniValue getblockstats(const JSONRPCRequest& request)
pindex = chainActive[height];
} else {
const std::string strHash = request.params[0].get_str();
const uint256 hash(uint256S(strHash));
const uint256 hash(ParseHashV(request.params[0], "hash_or_height"));
pindex = LookupBlockIndex(hash);
if (!pindex) {
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Block not found");

4
src/rpc/mining.cpp
View file

@ -247,7 +247,7 @@ static UniValue prioritisetransaction(const JSONRPCRequest& request)
LOCK(cs_main);
uint256 hash = ParseHashStr(request.params[0].get_str(), "txid");
uint256 hash(ParseHashV(request.params[0], "txid"));
CAmount nAmount = request.params[2].get_int64();
if (!(request.params[1].isNull() || request.params[1].get_real() == 0)) {
@ -455,7 +455,7 @@ static 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

6
src/rpc/rawtransaction.cpp
View file

@ -226,9 +226,7 @@ static UniValue gettxoutproof(const JSONRPCRequest& request)
UniValue txids = request.params[0].get_array();
for (unsigned int idx = 0; idx < txids.size(); idx++) {
const UniValue& txid = txids[idx];
if (txid.get_str().length() != 64 || !IsHex(txid.get_str()))
throw JSONRPCError(RPC_INVALID_PARAMETER, std::string("Invalid txid ")+txid.get_str());
uint256 hash(uint256S(txid.get_str()));
uint256 hash(ParseHashV(txid, "txid"));
if (setTxids.count(hash))
throw JSONRPCError(RPC_INVALID_PARAMETER, std::string("Invalid parameter, duplicated txid: ")+txid.get_str());
setTxids.insert(hash);
@ -239,7 +237,7 @@ static UniValue gettxoutproof(const JSONRPCRequest& request)
uint256 hashBlock;
if (!request.params[1].isNull()) {
LOCK(cs_main);
hashBlock = uint256S(request.params[1].get_str());
hashBlock = ParseHashV(request.params[1], "blockhash");
pblockindex = LookupBlockIndex(hashBlock);
if (!pblockindex) {
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Block not found");

12
src/rpc/server.cpp
View file

@ -116,16 +116,12 @@ CAmount AmountFromValue(const UniValue& value)
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)
{

3
src/wallet/rpcdump.cpp
View file

@ -415,8 +415,7 @@ UniValue removeprunedfunds(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"));
std::vector<uint256> vHash;
vHash.push_back(hash);
std::vector<uint256> vHashOut;

20
src/wallet/rpcwallet.cpp
View file

@ -1638,9 +1638,8 @@ static UniValue listsinceblock(const JSONRPCRequest& request)
isminefilter filter = ISMINE_SPENDABLE;
if (!request.params[0].isNull() && !request.params[0].get_str().empty()) {
uint256 blockId;
uint256 blockId(ParseHashV(request.params[0], "blockhash"));
blockId.SetHex(request.params[0].get_str());
paltindex = pindex = LookupBlockIndex(blockId);
if (!pindex) {
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Block not found");
@ -1768,8 +1767,7 @@ static 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;
if(!request.params[1].isNull())
@ -1836,8 +1834,7 @@ static 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");
@ -2241,17 +2238,13 @@ static 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()) {
@ -3176,8 +3169,7 @@ static UniValue bumpfee(const JSONRPCRequest& request)
}
RPCTypeCheck(request.params, {UniValue::VSTR, UniValue::VOBJ});
uint256 hash;
hash.SetHex(request.params[0].get_str());
uint256 hash(ParseHashV(request.params[0], "txid"));
// optional parameters
CAmount totalFee = 0;

3
test/functional/mining_prioritisetransaction.py
View file

@ -29,7 +29,8 @@ class PrioritiseTransactionTest(BitcoinTestFramework):
assert_raises_rpc_error(-1, "prioritisetransaction", self.nodes[0].prioritisetransaction, '', 0, 0, 0)
# Test `prioritisetransaction` invalid `txid`
assert_raises_rpc_error(-1, "txid must be hexadecimal string", self.nodes[0].prioritisetransaction, txid='foo', fee_delta=0)
assert_raises_rpc_error(-8, "txid must be of length 64 (not 3, for 'foo')", self.nodes[0].prioritisetransaction, txid='foo', fee_delta=0)
assert_raises_rpc_error(-8, "txid must be hexadecimal string (not 'Zd1d4e24ed99057e84c3f80fd8fbec79ed9e1acee37da269356ecea000000000')", self.nodes[0].prioritisetransaction, txid='Zd1d4e24ed99057e84c3f80fd8fbec79ed9e1acee37da269356ecea000000000', fee_delta=0)
# Test `prioritisetransaction` invalid `dummy`
txid = '1d1d4e24ed99057e84c3f80fd8fbec79ed9e1acee37da269356ecea000000000'

2
test/functional/p2p_compactblocks.py
View file

@ -293,7 +293,7 @@ class CompactBlocksTest(BitcoinTestFramework):
block_hash = int(node.generate(1)[0], 16)
# Store the raw block in our internal format.
block = FromHex(CBlock(), node.getblock("%02x" % block_hash, False))
block = FromHex(CBlock(), node.getblock("%064x" % block_hash, False))
for tx in block.vtx:
tx.calc_sha256()
block.rehash()

2
test/functional/p2p_sendheaders.py
View file

@ -402,7 +402,7 @@ class SendHeadersTest(BitcoinTestFramework):
block_time += 9
fork_point = self.nodes[0].getblock("%02x" % new_block_hashes[0])["previousblockhash"]
fork_point = self.nodes[0].getblock("%064x" % new_block_hashes[0])["previousblockhash"]
fork_point = int(fork_point, 16)
# Use getblocks/getdata

8
test/functional/rpc_blockchain.py
View file

@ -125,7 +125,9 @@ class BlockchainTest(BitcoinTestFramework):
# Test `getchaintxstats` invalid `blockhash`
assert_raises_rpc_error(-1, "JSON value is not a string as expected", self.nodes[0].getchaintxstats, blockhash=0)
assert_raises_rpc_error(-5, "Block not found", self.nodes[0].getchaintxstats, blockhash='0')
assert_raises_rpc_error(-8, "blockhash must be of length 64 (not 1, for '0')", self.nodes[0].getchaintxstats, blockhash='0')
assert_raises_rpc_error(-8, "blockhash must be hexadecimal string (not 'ZZZ0000000000000000000000000000000000000000000000000000000000000')", self.nodes[0].getchaintxstats, blockhash='ZZZ0000000000000000000000000000000000000000000000000000000000000')
assert_raises_rpc_error(-5, "Block not found", self.nodes[0].getchaintxstats, blockhash='0000000000000000000000000000000000000000000000000000000000000000')
blockhash = self.nodes[0].getblockhash(200)
self.nodes[0].invalidateblock(blockhash)
assert_raises_rpc_error(-8, "Block is not in main chain", self.nodes[0].getchaintxstats, blockhash=blockhash)
@ -203,7 +205,9 @@ class BlockchainTest(BitcoinTestFramework):
def _test_getblockheader(self):
node = self.nodes[0]
assert_raises_rpc_error(-5, "Block not found", node.getblockheader, "nonsense")
assert_raises_rpc_error(-8, "hash must be of length 64 (not 8, for 'nonsense')", node.getblockheader, "nonsense")
assert_raises_rpc_error(-8, "hash must be hexadecimal string (not 'ZZZ7bb8b1697ea987f3b223ba7819250cae33efacb068d23dc24859824a77844')", node.getblockheader, "ZZZ7bb8b1697ea987f3b223ba7819250cae33efacb068d23dc24859824a77844")
assert_raises_rpc_error(-5, "Block not found", node.getblockheader, "0cf7bb8b1697ea987f3b223ba7819250cae33efacb068d23dc24859824a77844")
besthash = node.getbestblockhash()
secondbesthash = node.getblockhash(199)

12
test/functional/rpc_rawtransaction.py
View file

@ -80,8 +80,9 @@ class RawTransactionsTest(BitcoinTestFramework):
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}], {})
@ -224,9 +225,10 @@ class RawTransactionsTest(BitcoinTestFramework):
# 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)

8
test/functional/rpc_txoutproof.py
View file

@ -66,12 +66,18 @@ class MerkleBlockTest(BitcoinTestFramework):
txid_spent = txin_spent["txid"]
txid_unspent = txid1 if txin_spent["txid"] != txid1 else txid2
# Invalid txids
assert_raises_rpc_error(-8, "txid must be of length 64 (not 32, for '00000000000000000000000000000000')", self.nodes[2].gettxoutproof, ["00000000000000000000000000000000"], blockhash)
assert_raises_rpc_error(-8, "txid must be hexadecimal string (not 'ZZZ0000000000000000000000000000000000000000000000000000000000000')", self.nodes[2].gettxoutproof, ["ZZZ0000000000000000000000000000000000000000000000000000000000000"], blockhash)
# Invalid blockhashes
assert_raises_rpc_error(-8, "blockhash must be of length 64 (not 32, for '00000000000000000000000000000000')", self.nodes[2].gettxoutproof, [txid_spent], "00000000000000000000000000000000")
assert_raises_rpc_error(-8, "blockhash must be hexadecimal string (not 'ZZZ0000000000000000000000000000000000000000000000000000000000000')", self.nodes[2].gettxoutproof, [txid_spent], "ZZZ0000000000000000000000000000000000000000000000000000000000000")
# We can't find the block from a fully-spent tx
assert_raises_rpc_error(-5, "Transaction not yet in block", self.nodes[2].gettxoutproof, [txid_spent])
# We can get the proof if we specify the block
assert_equal(self.nodes[2].verifytxoutproof(self.nodes[2].gettxoutproof([txid_spent], blockhash)), [txid_spent])
# We can't get the proof if we specify a non-existent block
assert_raises_rpc_error(-5, "Block not found", self.nodes[2].gettxoutproof, [txid_spent], "00000000000000000000000000000000")
assert_raises_rpc_error(-5, "Block not found", self.nodes[2].gettxoutproof, [txid_spent], "0000000000000000000000000000000000000000000000000000000000000000")
# We can get the proof if the transaction is unspent
assert_equal(self.nodes[2].verifytxoutproof(self.nodes[2].gettxoutproof([txid_unspent])), [txid_unspent])
# We can get the proof if we provide a list of transactions and one of them is unspent. The ordering of the list should not matter.

8
test/functional/wallet_basic.py
View file

@ -134,9 +134,15 @@ class WalletTest(BitcoinTestFramework):
assert_equal([unspent_0], self.nodes[2].listlockunspent())
self.nodes[2].lockunspent(True, [unspent_0])
assert_equal(len(self.nodes[2].listlockunspent()), 0)
assert_raises_rpc_error(-8, "Invalid parameter, unknown transaction",
assert_raises_rpc_error(-8, "txid must be of length 64 (not 34, for '0000000000000000000000000000000000')",
self.nodes[2].lockunspent, False,
[{"txid": "0000000000000000000000000000000000", "vout": 0}])
assert_raises_rpc_error(-8, "txid must be hexadecimal string (not 'ZZZ0000000000000000000000000000000000000000000000000000000000000')",
self.nodes[2].lockunspent, False,
[{"txid": "ZZZ0000000000000000000000000000000000000000000000000000000000000", "vout": 0}])
assert_raises_rpc_error(-8, "Invalid parameter, unknown transaction",
self.nodes[2].lockunspent, False,
[{"txid": "0000000000000000000000000000000000000000000000000000000000000000", "vout": 0}])
assert_raises_rpc_error(-8, "Invalid parameter, vout index out of bounds",
self.nodes[2].lockunspent, False,
[{"txid": unspent_0["txid"], "vout": 999}])

4
test/functional/wallet_listsinceblock.py
View file

@ -53,8 +53,10 @@ class ListSinceBlockTest (BitcoinTestFramework):
"42759cde25462784395a337460bde75f58e73d3f08bd31fdc3507cbac856a2c4")
assert_raises_rpc_error(-5, "Block not found", self.nodes[0].listsinceblock,
"0000000000000000000000000000000000000000000000000000000000000000")
assert_raises_rpc_error(-5, "Block not found", self.nodes[0].listsinceblock,
assert_raises_rpc_error(-8, "blockhash must be of length 64 (not 11, for 'invalid-hex')", self.nodes[0].listsinceblock,
"invalid-hex")
assert_raises_rpc_error(-8, "blockhash must be hexadecimal string (not 'Z000000000000000000000000000000000000000000000000000000000000000')", self.nodes[0].listsinceblock,
"Z000000000000000000000000000000000000000000000000000000000000000")
def test_reorg(self):
'''