scriptpubkeyman fuzz target creates Base58 too large; Fails on 32-bit (armhf, ...)

maflcko commented at 8:12 am on December 19, 2025: member

Decoding should not behave differently on different architectures. See also the related commit 3789215f73466606eb111714f596a2a5e9bb1933.

However, the scriptpubkeyman fuzz target seems to create base58 strings so large that uint32_t overflows for calculations in base58.cpp.

This is a bit hard to reproduce, because 32-bit platforms are rare and most of them don’t support running the unsigned integer overflow sanitizer. So the possible options to reproduce are:

Run on armhf and manually annotate the code to detect the overflow
Run on i386 with the integer sanitizer (possibly via podman run -it --rm --platform linux/i386 'debian:trixie')
Run the integer sanitizer on any 64-bit platform and manually replace type in the affected line by uint32_t

Afterwards, the steps to reproduce are:

0export DEBIAN_FRONTEND=noninteractive && apt update && apt install curl wget htop git vim ccache -y && git clone https://github.com/bitcoin/bitcoin.git  --depth=1 ./b-c && cd b-c && apt install build-essential cmake pkg-config  python3-zmq libzmq3-dev libevent-dev libboost-dev libsqlite3-dev  systemtap-sdt-dev  libcapnp-dev capnproto  libqrencode-dev qt6-tools-dev qt6-l10n-tools qt6-base-dev  clang llvm libc++-dev libc++abi-dev   -y
1
2cmake -B ./bld-cmake -DCMAKE_BUILD_TYPE=Debug -DCMAKE_C_COMPILER='clang' -DCMAKE_CXX_COMPILER='clang++' -DSANITIZERS=undefined,integer,float-divide-by-zero --preset=dev-mode
3
4cmake --build ./bld-cmake --parallel  $(nproc)
5
6curl -fLO 'https://github.com/bitcoin-core/qa-assets/raw/b5ad78e070e4cf36beb415d7b490d948d70ba73f/fuzz_corpora/scriptpubkeyman/1cf91e0c6bfff9dafcd4db5b0ba36b1e906f4cf5'
7
8UBSAN_OPTIONS="suppressions=$(pwd)/test/sanitizer_suppressions/ubsan:print_stacktrace=1:halt_on_error=1:report_error_type=1" FUZZ=scriptpubkeyman ./bld-cmake/bin/fuzz ./1cf91e0c6bfff9dafcd4db5b0ba36b1e906f4cf5

The output will be something like:

 0/b-c/src/base58.cpp:54:28: runtime error: unsigned integer overflow: 8744655 * 733 cannot be represented in type 'size_t' (aka 'unsigned int')
 1    [#0](/bitcoin-bitcoin/0/) 0x5985f849 in DecodeBase58(char const*, std::vector<unsigned char, std::allocator<unsigned char>>&, int) /b-c/bld-cmake/src/./base58.cpp:54:28
 2    [#1](/bitcoin-bitcoin/1/) 0x59860a8b in DecodeBase58Check(char const*, std::vector<unsigned char, std::allocator<unsigned char>>&, int) /b-c/bld-cmake/src/./base58.cpp:148:10
 3    [#2](/bitcoin-bitcoin/2/) 0x5986093b in DecodeBase58Check(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&, std::vector<unsigned char, std::allocator<unsigned char>>&, int) /b-c/bld-cmake/src/./base58.cpp:168:12
 4    [#3](/bitcoin-bitcoin/3/) 0x599c4fc8 in DecodeSecret(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&) /b-c/bld-cmake/src/./key_io.cpp:217:9
 5    [#4](/bitcoin-bitcoin/4/) 0x59b05478 in (anonymous namespace)::ParsePubkeyInner(unsigned int, std::span<char const, 4294967295u> const&, (anonymous namespace)::ParseScriptContext, FlatSigningProvider&, bool&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>&) /b-c/bld-cmake/src/./script/descriptor.cpp:1789:20
 6    [#5](/bitcoin-bitcoin/5/) 0x59af4256 in (anonymous namespace)::ParsePubkey(unsigned int&, std::span<char const, 4294967295u> const&, (anonymous namespace)::ParseScriptContext, FlatSigningProvider&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>&) /b-c/bld-cmake/src/./script/descriptor.cpp:1978:16
 7    [#6](/bitcoin-bitcoin/6/) 0x59adc3af in (anonymous namespace)::ParseScript(unsigned int&, std::span<char const, 4294967295u>&, (anonymous namespace)::ParseScriptContext, FlatSigningProvider&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>&) /b-c/bld-cmake/src/./script/descriptor.cpp:2148:24
 8    [#7](/bitcoin-bitcoin/7/) 0x59adbc65 in Parse(std::basic_string_view<char, std::char_traits<char>>, FlatSigningProvider&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>&, bool) /b-c/bld-cmake/src/./script/descriptor.cpp:2751:16
 9    [#8](/bitcoin-bitcoin/8/) 0x597ad5de in wallet::(anonymous namespace)::CreateWalletDescriptor(FuzzedDataProvider&) /b-c/bld-cmake/src/test/fuzz/./wallet/test/fuzz/scriptpubkeyman.cpp:73:61
10    [#9](/bitcoin-bitcoin/9/) 0x597ac489 in wallet::(anonymous namespace)::scriptpubkeyman_fuzz_target(std::span<unsigned char const, 4294967295u>) /b-c/bld-cmake/src/test/fuzz/./wallet/test/fuzz/scriptpubkeyman.cpp:104:22
11    [#10](/bitcoin-bitcoin/10/) 0x58950a8e in void std::__invoke_impl<void, void (*&)(std::span<unsigned char const, 4294967295u>), std::span<unsigned char const, 4294967295u>>(std::__invoke_other, void (*&)(std::span<unsigned char const, 4294967295u>), std::span<unsigned char const, 4294967295u>&&) /usr/lib/gcc/i686-linux-gnu/14/../../../../include/c++/14/bits/invoke.h:61:14
12    [#11](/bitcoin-bitcoin/11/) 0x589508eb in std::enable_if<is_invocable_r_v<void, void (*&)(std::span<unsigned char const, 4294967295u>), std::span<unsigned char const, 4294967295u>>, void>::type std::__invoke_r<void, void (*&)(std::span<unsigned char const, 4294967295u>), std::span<unsigned char const, 4294967295u>>(void (*&)(std::span<unsigned char const, 4294967295u>), std::span<unsigned char const, 4294967295u>&&) /usr/lib/gcc/i686-linux-gnu/14/../../../../include/c++/14/bits/invoke.h:111:2
13    [#12](/bitcoin-bitcoin/12/) 0x5895044b in std::_Function_handler<void (std::span<unsigned char const, 4294967295u>), void (*)(std::span<unsigned char const, 4294967295u>)>::_M_invoke(std::_Any_data const&, std::span<unsigned char const, 4294967295u>&&) /usr/lib/gcc/i686-linux-gnu/14/../../../../include/c++/14/bits/std_function.h:290:9
14    [#13](/bitcoin-bitcoin/13/) 0x5982ac95 in std::function<void (std::span<unsigned char const, 4294967295u>)>::operator()(std::span<unsigned char const, 4294967295u>) const /usr/lib/gcc/i686-linux-gnu/14/../../../../include/c++/14/bits/std_function.h:591:9
15    [#14](/bitcoin-bitcoin/14/) 0x5981f0da in test_one_input(std::span<unsigned char const, 4294967295u>) /b-c/bld-cmake/src/test/fuzz/util/./test/fuzz/fuzz.cpp:88:5
16    [#15](/bitcoin-bitcoin/15/) 0x59821b80 in main /b-c/bld-cmake/src/test/fuzz/util/./test/fuzz/fuzz.cpp:271:13
17    [#16](/bitcoin-bitcoin/16/) 0xf75a8cc2  (/lib/i386-linux-gnu/libc.so.6+0x24cc2) (BuildId: 2dc5f2945fad35c1b07d1a5a32520b3c41afaa75)
18    [#17](/bitcoin-bitcoin/17/) 0xf75a8d87 in __libc_start_main (/lib/i386-linux-gnu/libc.so.6+0x24d87) (BuildId: 2dc5f2945fad35c1b07d1a5a32520b3c41afaa75)
19    [#18](/bitcoin-bitcoin/18/) 0x58917db6 in _start (/b-c/bld-cmake/bin/fuzz+0x235ddb6) (BuildId: 7d8d83a77923f14e99c0de64acbc5f5bfc2cce9b)
20
21SUMMARY: UndefinedBehaviorSanitizer: unsigned-integer-overflow /b-c/src/base58.cpp:54:28

Obviously, it would be trivial to fix this by using the same type on 64-bit and 32-bit:

 0    refactor: Use uint64_t over size_t for multiplications in DecodeBase58
 1
 2diff --git a/src/base58.cpp b/src/base58.cpp
 3index 19a5bd3e55..5364b8276b 100644
 4--- a/src/base58.cpp
 5+++ b/src/base58.cpp
 6@@ -51,7 +51,7 @@ static const int8_t mapBase58[256] = {
 7         psz++;
 8     }
 9     // Allocate enough space in big-endian base256 representation.
10-    int size = strlen(psz) * 733 /1000 + 1; // log(58) / log(256), rounded up.
11+    int size(uint64_t{std::strlen(psz)} * 733 / 1000 + 1); // log(58) / log(256), rounded up.
12     std::vector<unsigned char> b256(size);
13     // Process the characters.
14     static_assert(std::size(mapBase58) == 256, "mapBase58.size() should be 256"); // guarantee not out of range

However, this seems like the wrong fix. A string of size 8744655 seems a bit large and should probably be rejected by the wallet code (if applicable), or by the fuzz target, if the condition is unreachable in a production wallet.

maflcko added the label Wallet on Dec 19, 2025

brunoerg commented at 4:59 pm on December 26, 2025: contributor

The scriptpubkeyman target creates a wallet based on a descriptor from the input. So it basically creates random descriptors which is leading to create this specific case where it’s a pk() fragment with a super large content into it. Besides this overflow issue I think these large descriptors are unproductive and makes the target slower, so could we reject large descriptors in the fuzz target?

maflcko commented at 5:15 pm on December 26, 2025: member

could we reject large descriptors in the fuzz target?

Yeah, that makes sense. I wonder if there is ever a use-case to expand any immediately following %ij%kl in the GetDescriptor helper. (Assuming this is what happens here). If no use-case exists, a requirement could be added to have any character in-between.

A hacky one-line diff to achieve that could be:

 0diff --git a/src/test/fuzz/util/descriptor.cpp b/src/test/fuzz/util/descriptor.cpp
 1index 9e52e990a2..debd6a43fd 100644
 2--- a/src/test/fuzz/util/descriptor.cpp
 3+++ b/src/test/fuzz/util/descriptor.cpp
 4@@ -62,7 +62,7 @@ std::optional<std::string> MockedDescriptorConverter::GetDescriptor(std::string_
 5             if (i + 3 >= mocked_desc.size()) return {};
 6             if (const auto idx = IdxFromHex(mocked_desc.substr(i + 1, 2))) {
 7                 desc += keys_str[*idx];
 8-                i += 3;
 9+                i += 4;
10             } else {
11                 return {};
12             }

I haven’t tried that, and if it works, it will need an explanatory comment.

brunoerg commented at 3:30 pm on December 29, 2025: contributor

I wonder if there is ever a use-case to expand any immediately following %ij%kl in the GetDescriptor helper. (Assuming this is what happens here). If no use-case exists, a requirement could be added to have any character in-between.

I think there is no use-case for expanding it, keys typically need to be separated by something so your suggestion makes sense. Perhaps a comment that explains it could be:

 0diff --git a/src/test/fuzz/util/descriptor.cpp b/src/test/fuzz/util/descriptor.cpp
 1index 9e52e990a2..7c5e43206a 100644
 2--- a/src/test/fuzz/util/descriptor.cpp
 3+++ b/src/test/fuzz/util/descriptor.cpp
 4@@ -62,7 +62,8 @@ std::optional<std::string> MockedDescriptorConverter::GetDescriptor(std::string_
 5             if (i + 3 >= mocked_desc.size()) return {};
 6             if (const auto idx = IdxFromHex(mocked_desc.substr(i + 1, 2))) {
 7                 desc += keys_str[*idx];
 8-                i += 3;
 9+               // Consume %ij (3 chars) + 1 separator. Prevents invalid consecutive placeholders like %00%01
10+                i += 4;
11             } else {

maflcko commented at 12:08 pm on December 30, 2025: member

I guess my above diff may not fully fix long strings and requiring a separator like , or ) after any %ij may also be needed.

brunoerg commented at 12:34 pm on December 30, 2025: contributor

I still think we should reject long strings in the target anyway. I think it’s maybe the only way to avoid these large descriptors that are improductive and make the target slower.

maflcko commented at 12:59 pm on December 30, 2025: member

The rpc target has base58 strings capped at max_base58_bytes_length{64}. Not sure if we want to limit all descriptors to this size.

I think to check that each inner fragment string is less than maximum meaningful byte length, could make sense.

scriptpubkeyman fuzz target creates Base58 too large; Fails on 32-bit (armhf, …) #34110