From e62f0c71f10def124b1c1219d790cef246a32c3e Mon Sep 17 00:00:00 2001 From: Sebastian Falbesoner Date: Sun, 29 Mar 2020 16:45:13 +0200 Subject: [PATCH 1/3] rpc: fix {sign,message}verify RPC errors for invalid address/signature --- src/rpc/misc.cpp | 4 ++-- src/wallet/rpcwallet.cpp | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/rpc/misc.cpp b/src/rpc/misc.cpp index 0c982317f5..e8f8a9837f 100644 --- a/src/rpc/misc.cpp +++ b/src/rpc/misc.cpp @@ -300,11 +300,11 @@ static RPCHelpMan verifymessage() switch (MessageVerify(strAddress, strSign, strMessage)) { case MessageVerificationResult::ERR_INVALID_ADDRESS: - throw JSONRPCError(RPC_TYPE_ERROR, "Invalid address"); + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid address"); case MessageVerificationResult::ERR_ADDRESS_NO_KEY: throw JSONRPCError(RPC_TYPE_ERROR, "Address does not refer to key"); case MessageVerificationResult::ERR_MALFORMED_SIGNATURE: - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Malformed base64 encoding"); + throw JSONRPCError(RPC_TYPE_ERROR, "Malformed base64 encoding"); case MessageVerificationResult::ERR_PUBKEY_NOT_RECOVERED: case MessageVerificationResult::ERR_NOT_SIGNED: return false; diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 17512265b5..8644849028 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -602,7 +602,7 @@ static UniValue signmessage(const JSONRPCRequest& request) CTxDestination dest = DecodeDestination(strAddress); if (!IsValidDestination(dest)) { - throw JSONRPCError(RPC_TYPE_ERROR, "Invalid address"); + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid address"); } const PKHash *pkhash = boost::get(&dest); From 9e399b9b2d386b28c0c0ff59fc75d31dbec31d9c Mon Sep 17 00:00:00 2001 From: Sebastian Falbesoner Date: Sun, 29 Mar 2020 18:28:46 +0200 Subject: [PATCH 2/3] test: check parameter validity in rpc_signmessage.py checks parameter validity and error codes for the related RPCs: - signmessagewithprivkey - signmessage - verifymessage --- test/functional/rpc_signmessage.py | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/test/functional/rpc_signmessage.py b/test/functional/rpc_signmessage.py index 0cb3ce4215..1c71732a61 100755 --- a/test/functional/rpc_signmessage.py +++ b/test/functional/rpc_signmessage.py @@ -5,7 +5,10 @@ """Test RPC commands for signing and verifying messages.""" from test_framework.test_framework import BitcoinTestFramework -from test_framework.util import assert_equal +from test_framework.util import ( + assert_equal, + assert_raises_rpc_error, +) class SignMessagesTest(BitcoinTestFramework): def set_test_params(self): @@ -38,5 +41,22 @@ class SignMessagesTest(BitcoinTestFramework): assert not self.nodes[0].verifymessage(other_address, signature, message) assert not self.nodes[0].verifymessage(address, other_signature, message) + self.log.info('test parameter validity and error codes') + # signmessage(withprivkey) have two required parameters + for num_params in [0, 1, 3, 4, 5]: + param_list = ["dummy"]*num_params + assert_raises_rpc_error(-1, "signmessagewithprivkey", self.nodes[0].signmessagewithprivkey, *param_list) + assert_raises_rpc_error(-1, "signmessage", self.nodes[0].signmessage, *param_list) + # verifymessage has three required parameters + for num_params in [0, 1, 2, 4, 5]: + param_list = ["dummy"]*num_params + assert_raises_rpc_error(-1, "verifymessage", self.nodes[0].verifymessage, *param_list) + # invalid key or address provided + assert_raises_rpc_error(-5, "Invalid private key", self.nodes[0].signmessagewithprivkey, "invalid_key", message) + assert_raises_rpc_error(-5, "Invalid address", self.nodes[0].signmessage, "invalid_addr", message) + assert_raises_rpc_error(-5, "Invalid address", self.nodes[0].verifymessage, "invalid_addr", signature, message) + # malformed signature provided + assert_raises_rpc_error(-3, "Malformed base64 encoding", self.nodes[0].verifymessage, self.nodes[0].getnewaddress(), "invalid_sig", message) + if __name__ == '__main__': SignMessagesTest().main() From a5cfb40e27bd281354bd0d14d91f83efb6bfce9f Mon Sep 17 00:00:00 2001 From: Sebastian Falbesoner Date: Sat, 29 Aug 2020 10:41:23 +0200 Subject: [PATCH 3/3] doc: release note for changed {sign,verify}message error codes --- doc/release-notes-18466.md | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 doc/release-notes-18466.md diff --git a/doc/release-notes-18466.md b/doc/release-notes-18466.md new file mode 100644 index 0000000000..e46d5064a3 --- /dev/null +++ b/doc/release-notes-18466.md @@ -0,0 +1,10 @@ +Low-level RPC changes +--------------------- + +- Error codes have been updated to be more accurate for the following error cases (#18466): + - `signmessage` now returns RPC_INVALID_ADDRESS_OR_KEY (-5) if the + passed address is invalid. Previously returned RPC_TYPE_ERROR (-3). + - `verifymessage` now returns RPC_INVALID_ADDRESS_OR_KEY (-5) if the + passed address is invalid. Previously returned RPC_TYPE_ERROR (-3). + - `verifymessage` now returns RPC_TYPE_ERROR (-3) if the passed signature + is malformed. Previously returned RPC_INVALID_ADDRESS_OR_KEY (-5).