Merge bitcoin/bitcoin#34109: refactor: Use uint64_t over size_t for serialize corruption check in fees.dat

fa1d17d56c refactor: Use uint64_t over size_t for serialize corruption check in fees.dat (MarcoFalke)

Pull request description:

  Serialization should not behave differently on different architectures. See also the related commit 3789215f73.

  However, on fees.dat file corruption, 32-bit builds may run into an unsigned integer overflow and report the wrong corruption reason, or may even silently continue after the corruption.

  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 '6074731370'

  UBSAN_OPTIONS="suppressions=$(pwd)/test/sanitizer_suppressions/ubsan:print_stacktrace=1:halt_on_error=1:report_error_type=1" FUZZ=policy_estimator_io ./bld-cmake/bin/fuzz ./607473137013139e3676e30ec4b29639e673fa9b
  ```

  The output will be something like:

  ```
  /b-c/src/policy/fees/block_policy_estimator.cpp:448:25: runtime error: unsigned integer overflow: 346685954 * 219 cannot be represented in type 'unsigned int'
      #0 0x5b0b1bbe in TxConfirmStats::Read(AutoFile&, unsigned int) /b-c/bld-cmake/src/./policy/fees/block_policy_estimator.cpp:448:25
      #1 0x5b0b7d3f in CBlockPolicyEstimator::Read(AutoFile&) /b-c/bld-cmake/src/./policy/fees/block_policy_estimator.cpp:1037:29
      #2 0x592a9783 in policy_estimator_io_fuzz_target(std::span<unsigned char const, 4294967295u>) /b-c/bld-cmake/src/test/fuzz/./test/fuzz/policy_estimator_io.cpp:32:32
      #3 0x5896ba8e 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
      #4 0x5896b8eb 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
      #5 0x5896b44b 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
      #6 0x59845c95 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
      #7 0x5983a0da in test_one_input(std::span<unsigned char const, 4294967295u>) /b-c/bld-cmake/src/test/fuzz/util/./test/fuzz/fuzz.cpp:88:5
      #8 0x5983cb80 in main /b-c/bld-cmake/src/test/fuzz/util/./test/fuzz/fuzz.cpp:271:13
      #9 0xf75aecc2  (/lib/i386-linux-gnu/libc.so.6+0x24cc2) (BuildId: 2dc5f2945fad35c1b07d1a5a32520b3c41afaa75)
      #10 0xf75aed87 in __libc_start_main (/lib/i386-linux-gnu/libc.so.6+0x24d87) (BuildId: 2dc5f2945fad35c1b07d1a5a32520b3c41afaa75)
      #11 0x58932db6 in _start (/b-c/bld-cmake/bin/fuzz+0x235ddb6) (BuildId: 7d8d83a77923f14e99c0de64acbc5f5bfc2cce9b)

  SUMMARY: UndefinedBehaviorSanitizer: unsigned-integer-overflow /b-c/src/policy/fees/block_policy_estimator.cpp:448:25
  ```

  Note: This is marked a "refactor", because the code change does not affect 64-bit builds, and on the still remaining rare 32-bit builds today it is extremely unlikely to happen in production.

ACKs for top commit:
  bensig:
    ACK fa1d17d56c
  ismaelsadeeq:
    utACK fa1d17d56c
  luke-jr:
    Also, utACK fa1d17d56c as an improvement.

Tree-SHA512: 696bf8e0dbe4777c84cb90e313c7f8f9ee90d4b3e64de1222f8472b2d9d0f3a0f6f027fda743dd6ca8c6aab94f404db7a65bb562a76000d9c33a8a39de28d8d4
This commit is contained in:
merge-script
2026-01-14 09:18:36 +00:00

View File

@@ -423,7 +423,7 @@ void TxConfirmStats::Read(AutoFile& filein, size_t numBuckets)
// Read data file and do some very basic sanity checking
// buckets and bucketMap are not updated yet, so don't access them
// If there is a read failure, we'll just discard this entire object anyway
size_t maxConfirms, maxPeriods;
uint64_t maxConfirms, maxPeriods;
// The current version will store the decay with each individual TxConfirmStats and also keep a scale factor
filein >> Using<EncodedDoubleFormatter>(decay);