From bb4d3e9b970be2a8de3e146623801fc8cbbeb0c7 Mon Sep 17 00:00:00 2001 From: Samuel Dobson Date: Tue, 23 Nov 2021 11:13:24 +1300 Subject: Address review comments for Bech32 error validation --- src/bech32.cpp | 2 +- src/key_io.cpp | 13 ++++++++----- test/functional/rpc_invalid_address_message.py | 10 +++++----- 3 files changed, 14 insertions(+), 11 deletions(-) diff --git a/src/bech32.cpp b/src/bech32.cpp index ea44480a6c..ec69965d07 100644 --- a/src/bech32.cpp +++ b/src/bech32.cpp @@ -391,7 +391,7 @@ void push_range(int from, int to, std::vector& vec) } } -/** Return index of first invalid character in a Bech32 string. */ +/** Return indices of invalid characters in a Bech32 string. */ bool CheckCharacters(const std::string& str, std::vector& errors) { bool lower = false, upper = false; for (size_t i = 0; i < str.size(); ++i) { diff --git a/src/key_io.cpp b/src/key_io.cpp index 6908c5ea52..c89493e29d 100644 --- a/src/key_io.cpp +++ b/src/key_io.cpp @@ -102,17 +102,20 @@ CTxDestination DecodeDestination(const std::string& str, const CChainParams& par return ScriptHash(hash); } - if (!std::equal(script_prefix.begin(), script_prefix.end(), data.begin()) && - !std::equal(pubkey_prefix.begin(), pubkey_prefix.end(), data.begin())) { - error_str = "Invalid prefix for Base58-encoded address"; - } else { + // If the prefix of data matches either the script or pubkey prefix, the length must have been wrong + if ((data.size() >= script_prefix.size() && + std::equal(script_prefix.begin(), script_prefix.end(), data.begin())) || + (data.size() >= pubkey_prefix.size() && + std::equal(pubkey_prefix.begin(), pubkey_prefix.end(), data.begin()))) { error_str = "Invalid length for Base58 address"; + } else { + error_str = "Invalid prefix for Base58-encoded address"; } return CNoDestination(); } else if (!is_bech32) { // Try Base58 decoding without the checksum, using a much larger max length if (!DecodeBase58(str, data, 100)) { - error_str = "Invalid HRP or Base58 character in address"; + error_str = "Not a valid Bech32 or Base58 encoding"; } else { error_str = "Invalid checksum or length of Base58 address"; } diff --git a/test/functional/rpc_invalid_address_message.py b/test/functional/rpc_invalid_address_message.py index 2567564f15..1aaeddbf00 100755 --- a/test/functional/rpc_invalid_address_message.py +++ b/test/functional/rpc_invalid_address_message.py @@ -61,7 +61,7 @@ class InvalidAddressErrorMessageTest(BitcoinTestFramework): def test_validateaddress(self): # Invalid Bech32 self.check_invalid(BECH32_INVALID_SIZE, 'Invalid Bech32 address data size') - self.check_invalid(BECH32_INVALID_PREFIX, 'Invalid HRP or Base58 character in address') + self.check_invalid(BECH32_INVALID_PREFIX, 'Not a valid Bech32 or Base58 encoding') self.check_invalid(BECH32_INVALID_BECH32, 'Version 1+ witness address must use Bech32m checksum') self.check_invalid(BECH32_INVALID_BECH32M, 'Version 0 witness address must use Bech32 checksum') self.check_invalid(BECH32_INVALID_VERSION, 'Invalid Bech32 address witness version') @@ -89,19 +89,19 @@ class InvalidAddressErrorMessageTest(BitcoinTestFramework): self.check_valid(BASE58_VALID) # Invalid address format - self.check_invalid(INVALID_ADDRESS, 'Invalid HRP or Base58 character in address') - self.check_invalid(INVALID_ADDRESS_2, 'Invalid HRP or Base58 character in address') + self.check_invalid(INVALID_ADDRESS, 'Not a valid Bech32 or Base58 encoding') + self.check_invalid(INVALID_ADDRESS_2, 'Not a valid Bech32 or Base58 encoding') def test_getaddressinfo(self): node = self.nodes[0] assert_raises_rpc_error(-5, "Invalid Bech32 address data size", node.getaddressinfo, BECH32_INVALID_SIZE) - assert_raises_rpc_error(-5, "Invalid HRP or Base58 character in address", node.getaddressinfo, BECH32_INVALID_PREFIX) + assert_raises_rpc_error(-5, "Not a valid Bech32 or Base58 encoding", node.getaddressinfo, BECH32_INVALID_PREFIX) assert_raises_rpc_error(-5, "Invalid prefix for Base58-encoded address", node.getaddressinfo, BASE58_INVALID_PREFIX) - assert_raises_rpc_error(-5, "Invalid HRP or Base58 character in address", node.getaddressinfo, INVALID_ADDRESS) + assert_raises_rpc_error(-5, "Not a valid Bech32 or Base58 encoding", node.getaddressinfo, INVALID_ADDRESS) def run_test(self): self.test_validateaddress() -- cgit v1.2.3