rpc: Fix invalid bech32 handling

4 files changed, 217 insertions, 6 deletions

diff --git a/src/key_io.cpp b/src/key_io.cpp
index 4659a59544..33499b0d23 100644
--- a/src/key_io.cpp
+++ b/src/key_io.cpp
@@ -124,7 +124,11 @@ CTxDestination DecodeDestination(const std::string& str, const CChainParams& par
 
     data.clear();
     const auto dec = bech32::Decode(str);
-    if ((dec.encoding == bech32::Encoding::BECH32 || dec.encoding == bech32::Encoding::BECH32M) && dec.data.size() > 0) {
+    if (dec.encoding == bech32::Encoding::BECH32 || dec.encoding == bech32::Encoding::BECH32M) {
+        if (dec.data.empty()) {
+            error_str = "Empty Bech32 data section";
+            return CNoDestination();
+        }
         // Bech32 decoding
         if (dec.hrp != params.Bech32HRP()) {
             error_str = strprintf("Invalid or unsupported prefix for Segwit (Bech32) address (expected %s, got %s).", params.Bech32HRP(), dec.hrp);
@@ -158,7 +162,7 @@ CTxDestination DecodeDestination(const std::string& str, const CChainParams& par
                     }
                 }
 
-                error_str = "Invalid Bech32 v0 address data size";
+                error_str = strprintf("Invalid Bech32 v0 address program size (%s byte), per BIP141", data.size());
                 return CNoDestination();
             }
 
@@ -175,7 +179,7 @@ CTxDestination DecodeDestination(const std::string& str, const CChainParams& par
             }
 
             if (data.size() < 2 || data.size() > BECH32_WITNESS_PROG_MAX_LEN) {
-                error_str = "Invalid Bech32 address data size";
+                error_str = strprintf("Invalid Bech32 address program size (%s byte)", data.size());
                 return CNoDestination();
             }
 
@@ -184,6 +188,9 @@ CTxDestination DecodeDestination(const std::string& str, const CChainParams& par
             std::copy(data.begin(), data.end(), unk.program);
             unk.length = data.size();
             return unk;
+        } else {
+            error_str = strprintf("Invalid padding in Bech32 data section");
+            return CNoDestination();
         }
     }
 
diff --git a/test/functional/rpc_invalid_address_message.py b/test/functional/rpc_invalid_address_message.py
index fd282a9bc1..59b45bbb9b 100755
--- a/test/functional/rpc_invalid_address_message.py
+++ b/test/functional/rpc_invalid_address_message.py
@@ -63,12 +63,12 @@ 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_SIZE, "Invalid Bech32 address program size (41 byte)")
         self.check_invalid(BECH32_INVALID_PREFIX, 'Invalid or unsupported Segwit (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')
-        self.check_invalid(BECH32_INVALID_V0_SIZE, 'Invalid Bech32 v0 address data size')
+        self.check_invalid(BECH32_INVALID_V0_SIZE, "Invalid Bech32 v0 address program size (21 byte), per BIP141")
         self.check_invalid(BECH32_TOO_LONG, 'Bech32 string too long', list(range(90, 108)))
         self.check_invalid(BECH32_ONE_ERROR, 'Invalid Bech32 checksum', [9])
         self.check_invalid(BECH32_TWO_ERRORS, 'Invalid Bech32 checksum', [22, 43])
@@ -105,7 +105,7 @@ class InvalidAddressErrorMessageTest(BitcoinTestFramework):
     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 Bech32 address program size (41 byte)", node.getaddressinfo, BECH32_INVALID_SIZE)
         assert_raises_rpc_error(-5, "Invalid or unsupported Segwit (Bech32) or Base58 encoding.", node.getaddressinfo, BECH32_INVALID_PREFIX)
         assert_raises_rpc_error(-5, "Invalid or unsupported Base58-encoded address.", node.getaddressinfo, BASE58_INVALID_PREFIX)
         assert_raises_rpc_error(-5, "Invalid or unsupported Segwit (Bech32) or Base58 encoding.", node.getaddressinfo, INVALID_ADDRESS)
diff --git a/test/functional/rpc_validateaddress.py b/test/functional/rpc_validateaddress.py
new file mode 100755
index 0000000000..2ce6b35d04
--- /dev/null
+++ b/test/functional/rpc_validateaddress.py
@@ -0,0 +1,203 @@
+#!/usr/bin/env python3
+# Copyright (c) 2023 The Bitcoin Core developers
+# Distributed under the MIT software license, see the accompanying
+# file COPYING or http://www.opensource.org/licenses/mit-license.php.
+"""Test validateaddress for main chain"""
+
+from test_framework.test_framework import BitcoinTestFramework
+
+from test_framework.util import assert_equal
+
+INVALID_DATA = [
+    # BIP 173
+    (
+        "tc1qw508d6qejxtdg4y5r3zarvary0c5xw7kg3g4ty",
+        "Invalid or unsupported Segwit (Bech32) or Base58 encoding.",  # Invalid hrp
+        [],
+    ),
+    ("bc1qw508d6qejxtdg4y5r3zarvary0c5xw7kv8f3t5", "Invalid Bech32 checksum", [41]),
+    (
+        "BC13W508D6QEJXTDG4Y5R3ZARVARY0C5XW7KN40WF2",
+        "Version 1+ witness address must use Bech32m checksum",
+        [],
+    ),
+    (
+        "bc1rw5uspcuh",
+        "Version 1+ witness address must use Bech32m checksum",  # Invalid program length
+        [],
+    ),
+    (
+        "bc10w508d6qejxtdg4y5r3zarvary0c5xw7kw508d6qejxtdg4y5r3zarvary0c5xw7kw5rljs90",
+        "Version 1+ witness address must use Bech32m checksum",  # Invalid program length
+        [],
+    ),
+    (
+        "BC1QR508D6QEJXTDG4Y5R3ZARVARYV98GJ9P",
+        "Invalid Bech32 v0 address program size (16 byte), per BIP141",
+        [],
+    ),
+    (
+        "tb1qrp33g0q5c5txsp9arysrx4k6zdkfs4nce4xj0gdcccefvpysxf3q0sL5k7",
+        "Invalid or unsupported Segwit (Bech32) or Base58 encoding.",  # tb1, Mixed case
+        [],
+    ),
+    (
+        "BC1QW508D6QEJXTDG4Y5R3ZARVARY0C5XW7KV8F3t4",
+        "Invalid character or mixed case",  # bc1, Mixed case, not in BIP 173 test vectors
+        [40],
+    ),
+    (
+        "bc1zw508d6qejxtdg4y5r3zarvaryvqyzf3du",
+        "Version 1+ witness address must use Bech32m checksum",  # Wrong padding
+        [],
+    ),
+    (
+        "tb1qrp33g0q5c5txsp9arysrx4k6zdkfs4nce4xj0gdcccefvpysxf3pjxtptv",
+        "Invalid or unsupported Segwit (Bech32) or Base58 encoding.",  # tb1, Non-zero padding in 8-to-5 conversion
+        [],
+    ),
+    ("bc1gmk9yu", "Empty Bech32 data section", []),
+    # BIP 350
+    (
+        "tc1p0xlxvlhemja6c4dqv22uapctqupfhlxm9h8z3k2e72q4k9hcz7vq5zuyut",
+        "Invalid or unsupported Segwit (Bech32) or Base58 encoding.",  # Invalid human-readable part
+        [],
+    ),
+    (
+        "bc1p0xlxvlhemja6c4dqv22uapctqupfhlxm9h8z3k2e72q4k9hcz7vqh2y7hd",
+        "Version 1+ witness address must use Bech32m checksum",  # Invalid checksum (Bech32 instead of Bech32m)
+        [],
+    ),
+    (
+        "tb1z0xlxvlhemja6c4dqv22uapctqupfhlxm9h8z3k2e72q4k9hcz7vqglt7rf",
+        "Invalid or unsupported Segwit (Bech32) or Base58 encoding.",  # tb1, Invalid checksum (Bech32 instead of Bech32m)
+        [],
+    ),
+    (
+        "BC1S0XLXVLHEMJA6C4DQV22UAPCTQUPFHLXM9H8Z3K2E72Q4K9HCZ7VQ54WELL",
+        "Version 1+ witness address must use Bech32m checksum",  # Invalid checksum (Bech32 instead of Bech32m)
+        [],
+    ),
+    (
+        "bc1qw508d6qejxtdg4y5r3zarvary0c5xw7kemeawh",
+        "Version 0 witness address must use Bech32 checksum",  # Invalid checksum (Bech32m instead of Bech32)
+        [],
+    ),
+    (
+        "tb1q0xlxvlhemja6c4dqv22uapctqupfhlxm9h8z3k2e72q4k9hcz7vq24jc47",
+        "Invalid or unsupported Segwit (Bech32) or Base58 encoding.",  # tb1, Invalid checksum (Bech32m instead of Bech32)
+        [],
+    ),
+    (
+        "bc1p38j9r5y49hruaue7wxjce0updqjuyyx0kh56v8s25huc6995vvpql3jow4",
+        "Invalid Base 32 character",  # Invalid character in checksum
+        [59],
+    ),
+    (
+        "BC130XLXVLHEMJA6C4DQV22UAPCTQUPFHLXM9H8Z3K2E72Q4K9HCZ7VQ7ZWS8R",
+        "Invalid Bech32 address witness version",
+        [],
+    ),
+    ("bc1pw5dgrnzv", "Invalid Bech32 address program size (1 byte)", []),
+    (
+        "bc1p0xlxvlhemja6c4dqv22uapctqupfhlxm9h8z3k2e72q4k9hcz7v8n0nx0muaewav253zgeav",
+        "Invalid Bech32 address program size (41 byte)",
+        [],
+    ),
+    (
+        "BC1QR508D6QEJXTDG4Y5R3ZARVARYV98GJ9P",
+        "Invalid Bech32 v0 address program size (16 byte), per BIP141",
+        [],
+    ),
+    (
+        "tb1p0xlxvlhemja6c4dqv22uapctqupfhlxm9h8z3k2e72q4k9hcz7vq47Zagq",
+        "Invalid or unsupported Segwit (Bech32) or Base58 encoding.",  # tb1, Mixed case
+        [],
+    ),
+    (
+        "bc1p0xlxvlhemja6c4dqv22uapctqupfhlxm9h8z3k2e72q4k9hcz7v07qwwzcrf",
+        "Invalid padding in Bech32 data section",  # zero padding of more than 4 bits
+        [],
+    ),
+    (
+        "tb1p0xlxvlhemja6c4dqv22uapctqupfhlxm9h8z3k2e72q4k9hcz7vpggkg4j",
+        "Invalid or unsupported Segwit (Bech32) or Base58 encoding.",  # tb1, Non-zero padding in 8-to-5 conversion
+        [],
+    ),
+    ("bc1gmk9yu", "Empty Bech32 data section", []),
+]
+VALID_DATA = [
+    # BIP 350
+    (
+        "BC1QW508D6QEJXTDG4Y5R3ZARVARY0C5XW7KV8F3T4",
+        "0014751e76e8199196d454941c45d1b3a323f1433bd6",
+    ),
+    # (
+    #   "tb1qrp33g0q5c5txsp9arysrx4k6zdkfs4nce4xj0gdcccefvpysxf3q0sl5k7",
+    #   "00201863143c14c5166804bd19203356da136c985678cd4d27a1b8c6329604903262",
+    # ),
+    (
+        "bc1qrp33g0q5c5txsp9arysrx4k6zdkfs4nce4xj0gdcccefvpysxf3qccfmv3",
+        "00201863143c14c5166804bd19203356da136c985678cd4d27a1b8c6329604903262",
+    ),
+    (
+        "bc1pw508d6qejxtdg4y5r3zarvary0c5xw7kw508d6qejxtdg4y5r3zarvary0c5xw7kt5nd6y",
+        "5128751e76e8199196d454941c45d1b3a323f1433bd6751e76e8199196d454941c45d1b3a323f1433bd6",
+    ),
+    ("BC1SW50QGDZ25J", "6002751e"),
+    ("bc1zw508d6qejxtdg4y5r3zarvaryvaxxpcs", "5210751e76e8199196d454941c45d1b3a323"),
+    # (
+    #   "tb1qqqqqp399et2xygdj5xreqhjjvcmzhxw4aywxecjdzew6hylgvsesrxh6hy",
+    #   "0020000000c4a5cad46221b2a187905e5266362b99d5e91c6ce24d165dab93e86433",
+    # ),
+    (
+        "bc1qqqqqp399et2xygdj5xreqhjjvcmzhxw4aywxecjdzew6hylgvses5wp4dt",
+        "0020000000c4a5cad46221b2a187905e5266362b99d5e91c6ce24d165dab93e86433",
+    ),
+    # (
+    #   "tb1pqqqqp399et2xygdj5xreqhjjvcmzhxw4aywxecjdzew6hylgvsesf3hn0c",
+    #   "5120000000c4a5cad46221b2a187905e5266362b99d5e91c6ce24d165dab93e86433",
+    # ),
+    (
+        "bc1pqqqqp399et2xygdj5xreqhjjvcmzhxw4aywxecjdzew6hylgvses7epu4h",
+        "5120000000c4a5cad46221b2a187905e5266362b99d5e91c6ce24d165dab93e86433",
+    ),
+    (
+        "bc1p0xlxvlhemja6c4dqv22uapctqupfhlxm9h8z3k2e72q4k9hcz7vqzk5jj0",
+        "512079be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798",
+    ),
+]
+
+
+class ValidateAddressMainTest(BitcoinTestFramework):
+    def set_test_params(self):
+        self.setup_clean_chain = True
+        self.chain = ""  # main
+        self.num_nodes = 1
+        self.extra_args = [["-prune=899"]] * self.num_nodes
+
+    def check_valid(self, addr, spk):
+        info = self.nodes[0].validateaddress(addr)
+        assert_equal(info["isvalid"], True)
+        assert_equal(info["scriptPubKey"], spk)
+        assert "error" not in info
+        assert "error_locations" not in info
+
+    def check_invalid(self, addr, error_str, error_locations):
+        res = self.nodes[0].validateaddress(addr)
+        assert_equal(res["isvalid"], False)
+        assert_equal(res["error"], error_str)
+        assert_equal(res["error_locations"], error_locations)
+
+    def test_validateaddress(self):
+        for (addr, error, locs) in INVALID_DATA:
+            self.check_invalid(addr, error, locs)
+        for (addr, spk) in VALID_DATA:
+            self.check_valid(addr, spk)
+
+    def run_test(self):
+        self.test_validateaddress()
+
+
+if __name__ == "__main__":
+    ValidateAddressMainTest().main()
diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py
index 3dc22ccde3..a6f1bbae7d 100755
--- a/test/functional/test_runner.py
+++ b/test/functional/test_runner.py
@@ -171,6 +171,7 @@ BASE_SCRIPTS = [
     'wallet_fast_rescan.py --descriptors',
     'interface_zmq.py',
     'rpc_invalid_address_message.py',
+    'rpc_validateaddress.py',
     'interface_bitcoin_cli.py --legacy-wallet',
     'interface_bitcoin_cli.py --descriptors',
     'feature_bind_extra.py',