blockstorage: simplify partial block read validation
What the developer wrote
Authored by Roman Zeyde
blockstorage: simplify partial block read validation
Use `SaturatingAdd` following https://github.com/bitcoin/bitcoin/pull/33657#discussion_r2610832092.
What changed, and why it matters
This is a tiny code cleanup in Bitcoin Core's block storage module. It replaces a manual check that prevents reading beyond the end of a raw block file with a simpler check using a helper called SaturatingAdd. The change appears to be a defensive simplification rather than a fix for a known vulnerability. There is no evidence in the commit or supplied references that this addresses a security issue or was reported by an outside researcher.
No immediate action required. Treat as routine hardening/refactoring. If reviewing, verify that SaturatingAdd behaves identically to the prior three-part check across the relevant integer ranges and that no caller relies on the old expression's short-circuit behavior.
Security signals we found
Input validation on untrusted REST offset/size parameters
Use of saturating arithmetic to avoid unsigned overflow in bounds check
Avoidance of logging for untrusted input (anti-DoS/logging consideration)
Evidence from the diff
In BlockManager::ReadRawBlock(), the validation of an untrusted offset/size pair from REST input is changed from size == 0 || offset >= blk_size || size > blk_size - offset to size == 0 || SaturatingAdd(offset, size) > blk_size. The two expressions are logically equivalent when no overflow occurs, because offset + size > blk_size is equivalent to size > blk_size - offset for non-negative values. SaturatingAdd prevents unsigned overflow from wrapping, making the intent clearer and slightly more robust. The comment explicitly notes the inputs come from untrusted REST input and that logging is intentionally avoided.
Changed components
src/node/blockstorage.cppBlockManager::ReadRawBlock()REST API raw block partial read pathInspect captured patch +1 / −1
diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp
index 22c8f619..e39b4163 100644
--- a/src/node/blockstorage.cpp
+++ b/src/node/blockstorage.cpp
@@ -1083,7 +1083,7 @@ BlockManager::ReadRawBlockResult BlockManager::ReadRawBlock(const FlatFilePos& p
if (block_part) {
const auto [offset, size]{*block_part};
- if (size == 0 || offset >= blk_size || size > blk_size - offset) {
+ if (size == 0 || SaturatingAdd(offset, size) > blk_size) {
return util::Unexpected{ReadRawError::BadPartRange}; // Avoid logging - offset/size come from untrusted REST input
}
filein.seek(offset, SEEK_CUR);
Why this scored 29/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.