fix: UB sanitizer in mempool estimator logging
What the developer wrote
Authored by rustaceanrob
fix: UB sanitizer in mempool estimator logging
The following is failing in CI, when a block has `m_height` of 64 bit
max:
```
SUMMARY: UndefinedBehaviorSanitizer: unsigned-integer-overflow /home/runner/work/_temp/src/policy/fees/mempool_estimator.cpp:210:62
MS: 0 ; base unit: 0000000000000000000000000000000000000000
0x1,0x0,0x0,0x0,0x3,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0x1,0x0,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0x0,0x0,0x26,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x2f,0x0,0x3,0x2,0x2,0x2,0x2,0x2,0x2,0x2,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x7a,0x3f,0x3f,0x0,0x0,0x2f,0x0,0x3,0x2,0x2,0x2,0x2,0x2,0x2,0x2,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x7a,0x3f,0x3f,0x3f,0xff,0xff,0xff,0xff,0xff,0x18,0x0,0x0,0x85,0x3f,0xff,0xff,0xff,0xff,0xff,0x18,0x0,0x0,0x85,0xd6,0x1,0x0,0x86,0x0,0x0,0x0,0x2a,0x0,0xff,0xff,0xff,
\001\000\000\000\003\377\377\377\377\377\377\377\377\001\000\377\377\377\377\377\377\377\377\000\000&\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000/\000\003\002\002\002\002\002\002\002\000\000\000\000\000\000\000z??\000\000/\000\003\002\002\002\002\002\002\002\000\000\000\000\000\000\000z???\377\377\377\377\377\030\000\000\205?\377\377\377\377\377\030\000\000\205\326\001\000\206\000\000\000*\000\377\377\377
artifact_prefix='./'; Test unit written to ./crash-b4333d1fe3993fe8610b86654e385682c23050b9
Base64: AQAAAAP//////////wEA//////////8AACYAAAAAAAAAAAAAAAAAAAAAAAAvAAMCAgICAgICAAAAAAAAAHo/PwAALwADAgICAgICAgAAAAAAAAB6Pz8///////8YAACFP///////GAAAhdYBAIYAAAAqAP///w==
⚠️ Failure generated from target with exit code 1: ['/home/runner/work/_temp/build_ ₿🧪_/bin/fuzz', '-runs=1', PosixPath('/home/runner/work/_temp/ci/scratch_ ₿🧪_/qa-assets/fuzz_corpora/policy_estimator_io')]
Check if using libFuzzer ... True
Command '['docker', 'exec', '--env', 'DANGER_RUN_CI_ON_HOST=1', '8100bf684275e706787e07f8ab94431926ba5184562c52bce95c932210f6f38f', '/home/runner/work/_temp/ci/test/03_test_script.sh']' returned non-zero exit status 1.
```
What changed, and why it matters
This commit fixes a logging statement in Bitcoin Core's fee estimator that could trigger an 'unsigned integer overflow' warning when the block height reaches the maximum possible 64-bit value. The code now uses a safe addition helper that prevents the overflow. It is a defensive fix for a sanitizer failure found during automated fuzz testing, not a user-exploitable vulnerability in normal operation.
No urgent action required. Treat as a routine code-quality/sanitizer fix. If running a node, update to a version containing this commit to avoid CI/test failures and maintain clean sanitizer coverage. No special mitigation is needed for production operation.
Security signals we found
UndefinedBehaviorSanitizer unsigned-integer-overflow triggered in CI fuzz run
Fix uses overflow-safe SaturatingAdd helper
Trigger condition requires block height at uint64_t max, which is not realistic on mainnet
Only affects logging and input validation path in fee estimator file read
Evidence from the diff
In MemPoolFeeRateEstimator::Read(), the code compared a stored block height against blocks[i-1].m_height + 1 and logged that value. If m_height is UINT64_MAX, the expression m_height + 1 overflows, which is defined behavior for unsigned integers in C++ but is flagged by UndefinedBehaviorSanitizer’s unsigned-integer-overflow check. The patch replaces the raw addition with SaturatingAdd() from util/overflow.h and uses the resulting expected_height variable in both the comparison and the LogWarning() call. This is a correctness/sanitizer fix rather than a consensus or remote-exploitable bug.
Changed components
src/policy/fees/mempool_estimator.cppMemPoolFeeRateEstimator::Read()Fee estimator block-height validation and loggingInspect captured patch +4 / −2
### src/policy/fees/mempool_estimator.cpp
@@ -18,6 +18,7 @@
#include <util/feefrac.h>
#include <util/fees.h>
#include <util/fs.h>
+#include <util/overflow.h>
#include <util/syserror.h>
#include <validation.h>
@@ -207,10 +208,11 @@ bool MemPoolFeeRateEstimator::Read(AutoFile& file)
return false;
}
for (size_t i = 1; i < blocks.size(); ++i) {
- if (blocks[i].m_height != blocks[i - 1].m_height + 1) {
+ const uint64_t expected_height{SaturatingAdd(blocks[i - 1].m_height, uint64_t{1})};
+ if (blocks[i].m_height != expected_height) {
LogWarning("%s: Non-consecutive block heights read, expected height %s but found %s; ignoring file",
FeeRateEstimatorTypeToString(FeeRateEstimatorType::MEMPOOL_POLICY),
- blocks[i - 1].m_height + 1, blocks[i].m_height);
+ expected_height, blocks[i].m_height);
return false;
}
}Why this scored 26/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.