From 2b71a9b030363d10f4d0e7b1f5453a054fbb5792 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Mon, 13 Dec 2021 17:13:28 +0100 Subject: [PATCH] Merge bitcoin/bitcoin#23755: rpc: Quote user supplied strings in error messages fa24a3df8796cbf4eeb35d950a4c848d605e5b22 rpc: Quote user supplied strings in error messages (MarcoFalke) Pull request description: I can't see a downside doing this and this fixes a fuzzing crash Background: This is a follow-up to commit 926fc2a0d4ff64cf2ff8e1dfa64eca2ebd24e090, which introduced the "starts_with-hack". Maybe an alternative to the hack would be to assign a unique error code to internal bugs? However, I think this can be done in an separate pull request and the changes here make sense even on their own. ACKs for top commit: fanquake: ACK fa24a3df8796cbf4eeb35d950a4c848d605e5b22 - to fix the fuzzers. Tree-SHA512: d998626406a64396a037a6d1fce22fce3dadb7567c2f9638e450ebe8fb8ae77d134e15dd02555326732208f698d77b0028bc62be9ceee9c43282b61fe95fccbd --- src/rpc/blockchain.cpp | 4 ++-- src/rpc/util.cpp | 2 +- test/functional/rpc_blockchain.py | 2 +- test/functional/rpc_getblockstats.py | 8 ++++---- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 16055175fb..28d075aaba 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -1377,7 +1377,7 @@ CoinStatsHashType ParseHashType(const std::string& hash_type_input) } else if (hash_type_input == "none") { return CoinStatsHashType::NONE; } else { - throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("%s is not a valid hash_type", hash_type_input)); + throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("'%s' is not a valid hash_type", hash_type_input)); } } @@ -2520,7 +2520,7 @@ static RPCHelpMan getblockstats() for (const std::string& stat : stats) { const UniValue& value = ret_all[stat]; if (value.isNull()) { - throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Invalid selected statistic %s", stat)); + throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Invalid selected statistic '%s'", stat)); } ret.pushKV(stat, value); } diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp index 3b02da632f..3741a1b9a9 100644 --- a/src/rpc/util.cpp +++ b/src/rpc/util.cpp @@ -255,7 +255,7 @@ CPubKey AddrToPubKey(const FillableSigningProvider& keystore, const std::string& } const PKHash *pkhash = std::get_if(&dest); if (!pkhash) { - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("%s does not refer to a key", addr_in)); + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("'%s' does not refer to a key", addr_in)); } CPubKey vchPubKey; if (!keystore.GetPubKey(ToKeyID(*pkhash), vchPubKey)) { diff --git a/test/functional/rpc_blockchain.py b/test/functional/rpc_blockchain.py index fedeb1f3d8..fb1c019769 100755 --- a/test/functional/rpc_blockchain.py +++ b/test/functional/rpc_blockchain.py @@ -299,7 +299,7 @@ class BlockchainTest(BitcoinTestFramework): assert 'muhash' not in r # Unknown hash_type raises an error - assert_raises_rpc_error(-8, "foohash is not a valid hash_type", node.gettxoutsetinfo, "foohash") + assert_raises_rpc_error(-8, "'foo hash' is not a valid hash_type", node.gettxoutsetinfo, "foo hash") def _test_getblockheader(self): self.log.info("Test getblockheader") diff --git a/test/functional/rpc_getblockstats.py b/test/functional/rpc_getblockstats.py index ebcf4e2ebb..19295b70e5 100755 --- a/test/functional/rpc_getblockstats.py +++ b/test/functional/rpc_getblockstats.py @@ -143,17 +143,17 @@ class GetblockstatsTest(BitcoinTestFramework): inv_sel_stat = 'asdfghjkl' inv_stats = [ [inv_sel_stat], - ['minfee' , inv_sel_stat], + ['minfee', inv_sel_stat], [inv_sel_stat, 'minfee'], ['minfee', inv_sel_stat, 'maxfee'], ] for inv_stat in inv_stats: - assert_raises_rpc_error(-8, 'Invalid selected statistic %s' % inv_sel_stat, + assert_raises_rpc_error(-8, f"Invalid selected statistic '{inv_sel_stat}'", self.nodes[0].getblockstats, hash_or_height=1, stats=inv_stat) # Make sure we aren't always returning inv_sel_stat as the culprit stat - assert_raises_rpc_error(-8, 'Invalid selected statistic aaa%s' % inv_sel_stat, - self.nodes[0].getblockstats, hash_or_height=1, stats=['minfee' , 'aaa%s' % inv_sel_stat]) + assert_raises_rpc_error(-8, f"Invalid selected statistic 'aaa{inv_sel_stat}'", + self.nodes[0].getblockstats, hash_or_height=1, stats=['minfee', f'aaa{inv_sel_stat}']) # Mainchain's genesis block shouldn't be found on regtest assert_raises_rpc_error(-5, 'Block not found', self.nodes[0].getblockstats, hash_or_height='000000000019d6689c085ae165831e934ff763ae46a2a6c172b3f1b60a8ce26f')