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:
export 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
cmake -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
cmake --build ./bld-cmake --parallel $(nproc)
curl -fLO 'https://github.com/bitcoin-core/qa-assets/raw/b5ad78e070e4cf36beb415d7b490d948d70ba73f/fuzz_corpora/scriptpubkeyman/1cf91e0c6bfff9dafcd4db5b0ba36b1e906f4cf5'
UBSAN_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:
/b-c/src/base58.cpp:54:28: runtime error: unsigned integer overflow: 8744655 * 733 cannot be represented in type 'size_t' (aka 'unsigned int')
[#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
[#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
[#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
[#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
[#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
[#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
[#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
[#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
[#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
[#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
[#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
[#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
[#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
[#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
[#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
[#15](/bitcoin-bitcoin/15/) 0x59821b80 in main /b-c/bld-cmake/src/test/fuzz/util/./test/fuzz/fuzz.cpp:271:13
[#16](/bitcoin-bitcoin/16/) 0xf75a8cc2 (/lib/i386-linux-gnu/libc.so.6+0x24cc2) (BuildId: 2dc5f2945fad35c1b07d1a5a32520b3c41afaa75)
[#17](/bitcoin-bitcoin/17/) 0xf75a8d87 in __libc_start_main (/lib/i386-linux-gnu/libc.so.6+0x24d87) (BuildId: 2dc5f2945fad35c1b07d1a5a32520b3c41afaa75)
[#18](/bitcoin-bitcoin/18/) 0x58917db6 in _start (/b-c/bld-cmake/bin/fuzz+0x235ddb6) (BuildId: 7d8d83a77923f14e99c0de64acbc5f5bfc2cce9b)
SUMMARY: 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:
refactor: Use uint64_t over size_t for multiplications in DecodeBase58
diff --git a/src/base58.cpp b/src/base58.cpp
index 19a5bd3e55..5364b8276b 100644
--- a/src/base58.cpp
+++ b/src/base58.cpp
@@ -51,7 +51,7 @@ static const int8_t mapBase58[256] = {
psz++;
}
// Allocate enough space in big-endian base256 representation.
- int size = strlen(psz) * 733 /1000 + 1; // log(58) / log(256), rounded up.
+ int size(uint64_t{std::strlen(psz)} * 733 / 1000 + 1); // log(58) / log(256), rounded up.
std::vector<unsigned char> b256(size);
// Process the characters.
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.
