Skip to content

Commit

Permalink
rpc: check and throw specific pubkey parsing errors in HexToPubKey
Browse files Browse the repository at this point in the history
In the helper `HexToPubKey`, check for three different causes of legacy
public key parsing errors (in this order):

    - pubkey is not a hex string
    - pubkey doesn't have a valid length (33 or 65 bytes) [NEW]
    - pubkey is cryptographically invalid, i.e. not on curve
      (`IsFullyValid` check)

and throw a specific error message for each one. Note that the error
code is identical for all of them (-5), so this doesn't break RPC API
compatibility.

The helper is currently used for the RPCs `createmultisig` and
`addmultisigaddress`. The length checks can be removed from the
call-sites and error message checks in the functional tests are adapted.
  • Loading branch information
theStack committed Feb 9, 2024
1 parent b2b2b1e commit 100e8a7
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 9 deletions.
6 changes: 1 addition & 5 deletions src/rpc/output_script.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -124,11 +124,7 @@ static RPCHelpMan createmultisig()
const UniValue& keys = request.params[1].get_array();
std::vector<CPubKey> pubkeys;
for (unsigned int i = 0; i < keys.size(); ++i) {
if (IsHex(keys[i].get_str()) && (keys[i].get_str().length() == 66 || keys[i].get_str().length() == 130)) {
pubkeys.push_back(HexToPubKey(keys[i].get_str()));
} else {
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("Invalid public key: %s\n.", keys[i].get_str()));
}
pubkeys.push_back(HexToPubKey(keys[i].get_str()));
}

// Get the output type
Expand Down
7 changes: 5 additions & 2 deletions src/rpc/util.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -180,11 +180,14 @@ std::string HelpExampleRpcNamed(const std::string& methodname, const RPCArgList&
CPubKey HexToPubKey(const std::string& hex_in)
{
if (!IsHex(hex_in)) {
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid public key: " + hex_in);
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Pubkey \"" + hex_in + "\" must be a hex string");
}
if (hex_in.length() != 66 && hex_in.length() != 130) {
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Pubkey \"" + hex_in + "\" must have a length of either 33 or 65 bytes");
}
CPubKey vchPubKey(ParseHex(hex_in));
if (!vchPubKey.IsFullyValid()) {
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid public key: " + hex_in);
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Pubkey \"" + hex_in + "\" must be cryptographically valid.");
}
return vchPubKey;
}
Expand Down
4 changes: 2 additions & 2 deletions test/functional/rpc_rawtransaction.py
Original file line number Diff line number Diff line change
Expand Up @@ -491,11 +491,11 @@ def raw_multisig_transaction_legacy_tests(self):
addr2Obj = self.nodes[2].getaddressinfo(addr2)

# Tests for createmultisig and addmultisigaddress
assert_raises_rpc_error(-5, "Invalid public key", self.nodes[0].createmultisig, 1, ["01020304"])
assert_raises_rpc_error(-5, 'Pubkey "01020304" must have a length of either 33 or 65 bytes', self.nodes[0].createmultisig, 1, ["01020304"])
# createmultisig can only take public keys
self.nodes[0].createmultisig(2, [addr1Obj['pubkey'], addr2Obj['pubkey']])
# addmultisigaddress can take both pubkeys and addresses so long as they are in the wallet, which is tested here
assert_raises_rpc_error(-5, "Invalid public key", self.nodes[0].createmultisig, 2, [addr1Obj['pubkey'], addr1])
assert_raises_rpc_error(-5, f'Pubkey "{addr1}" must be a hex string', self.nodes[0].createmultisig, 2, [addr1Obj['pubkey'], addr1])

mSigObj = self.nodes[2].addmultisigaddress(2, [addr1Obj['pubkey'], addr1])['address']

Expand Down

0 comments on commit 100e8a7

Please sign in to comment.