#35818 bloom: avoid undefined sizing calculations
https://github.com/bitcoin/bitcoin/pull/35818 · · +75/-4 in 4 files, 4 commits · labels: none
Goal
- Avoid division by zero and undefined floating-point conversions during bloom filter sizing
- Prevents crashes and undefined behavior when running unit tests and fuzz targets with edge-case inputs
This PR fixes undefined floating-point conversion and division by zero in the four-argument `CBloomFilter` constructor. It validates that the false-positive rate is within [0, 1], handles zero elements and zero false-positive rates explicitly, clamps the calculated filter size before converting from floating point to an unsigned integer, and expands unit and fuzz test coverage for boundary parameters.
Problem: The four-argument `CBloomFilter` constructor, used by unit tests and the fuzz target, divides by zero when given zero elements and causes undefined behavior when converting out-of-range floating-point results to `unsigned int`.
Category: Utilities (logging, arguments, libraries) (#36 of 66)
P4 · bug fix
- P4 because the fix prevents undefined behavior and crashes during bloom filter sizing
- Limited to edge cases in unit tests and fuzz targets rather than production node paths
Fixes floating-point undefined behavior and division by zero in CBloomFilter sizing, but the affected constructor is used exclusively in tests and fuzzing rather than production node paths.
Membership: Modifies CBloomFilter utility implementation and sizing calculations in src/common/bloom.cpp.
Factors: security/stability 1, bug 1, performance 0, user value 0, leverage 0
Reviewability: Ready
- Ready for review with passing CI and no blocking requests
There are no blocking reviewer requests, CI is passing, and rustaceanrob has code-review ACKed the current commit.
Author status: silent since 2026-08-05 after addressing reviewer suggestions and agreeing to follow-up work
Resolved concerns:
- rustaceanrob suggested moving the test-only constructor behind a test utility factory, but agreed that it should be deferred to a follow-up PR.
Agreement: Positive
- Full code review approval with no remaining objections (rustaceanrob)
- Agreed to defer moving test construction logic into a helper to a follow-up (rustaceanrob)
Positive: code review ACK from rustaceanrob with no objections
rustaceanrob reviewed and ACKed the commit after agreeing to defer architectural refactoring to a follow-up.
- rustaceanrob: Concept ACK and later 'Code review ACK c4302ddf80'
- rustaceanrob agreed with l0rinc that moving test construction into test utilities can happen in a follow-up
Review verdicts (DrahtBot): 1
- ACK: rustaceanrob
Files
64 lines under test/bench/ci.
- src/test/bloom_tests.cpp +51/-0
- src/common/bloom.cpp +11/-2
- src/test/fuzz/bloom_filter.cpp +11/-2
- src/common/bloom.h +2/-0
Card
This PR fixes undefined floating-point conversion and division by zero in the four-argument CBloomFilter constructor by validating false-positive rates and clamping size calculations before conversion. The affected constructor is only invoked in unit tests and fuzzing harnesses, meaning production bitcoind operations are unaffected. The PR also expands fuzz and unit test coverage across boundary conditions. Review is complete with a Code Review ACK from rustaceanrob and no outstanding objections.