Merge bitcoin/bitcoin#36321: net: cast vector size to avoid overflow, truncation, sign change
What the developer wrote
Authored by merge-script
Merge bitcoin/bitcoin#36321: net: cast vector size to avoid overflow, truncation, sign change
308cd670195d908fbd50caf76a8a215386121bd0 net: cast vector size to avoid overflow, truncation, sign change (Eugene Siegel)
Pull request description:
When running with `-fsanitize=integer` compiled, the following can error [here](https://github.com/bitcoin/bitcoin/blob/b3f846ec3e5c21b08779ac6c13475f3ab37e7d9c/src/net_processing.cpp#L3662):
```
SUMMARY: UndefinedBehaviorSanitizer: unsigned-integer-overflow /bitcoin/src/net_processing.cpp:3662:33
SUMMARY: UndefinedBehaviorSanitizer: implicit-signed-integer-truncation-or-sign-change /bitcoin/src/net_processing.cpp:3662:18
```
When `stop_index->nHeight` is less than `CFCHECKPT_INTERVAL`, the `headers` vector will be empty. This will just set the loop counter to -1 and never enter the loop, so this is harmless anyways. Fix this by casting `headers.size()` to `int`.
ACKs for top commit:
maflcko:
lgtm ACK 308cd670195d908fbd50caf76a8a215386121bd0
davidgumberg:
crACK https://github.com/bitcoin/bitcoin/commit/308cd670195d908fbd50caf76a8a215386121bd0
Tree-SHA512: 1074667b644e42507b32043e98a0541fff6de9a3711003a43b9874b31956a8e10efe65af89ed2b23322a7aa3df7d6a8606db5c1ab342f070a625b9e9650ae1fa
What changed, and why it matters
This is a one-line fix in Bitcoin Core's network code. It changes how the size of a list of block headers is converted to a signed integer inside a loop. Previously, if the list was empty, the conversion could trigger undefined-behavior warnings under special compiler sanitizers because subtracting 1 from an unsigned zero produced a very large positive number, which then got squeezed into a signed integer. The code already did not enter the loop in that case, so there was no practical exploit path. The patch simply makes the conversion explicit and clean.
No urgent action needed beyond applying the patch. The issue is a code-quality/UBSan cleanliness fix with no demonstrated security impact. Continue normal review and testing.
Security signals we found
UndefinedBehaviorSanitizer integer sanitizer warning addressed
Implicit signed/unsigned conversion in loop counter
Unsigned integer wraparound on empty vector size
One-line defensive cast in network processing code
Evidence from the diff
In src/net_processing.cpp, ProcessGetCFCheckPt iterates a headers vector with for (int i = headers.size() - 1; i >= 0; i--). Because headers.size() returns size_t (unsigned), when the vector is empty the expression headers.size() - 1 wraps to SIZE_MAX before being converted/truncated to int, triggering UBSan integer sanitizer errors (unsigned-integer-overflow and implicit-signed-integer-truncation-or-sign-change). The patch casts the size to int first: int(headers.size()) - 1. The loop guard i >= 0 already prevents entry when the result is -1, so behavior is harmless in practice.
Changed components
src/net_processing.cppPeerManagerImpl::ProcessGetCFCheckPtcompact block filter checkpoint header servingInspect captured patch +1 / −1
### src/net_processing.cpp
@@ -3659,7 +3659,7 @@ void PeerManagerImpl::ProcessGetCFCheckPt(CNode& node, Peer& peer, DataStream& v
// Populate headers.
const CBlockIndex* block_index = stop_index;
- for (int i = headers.size() - 1; i >= 0; i--) {
+ for (int i = int(headers.size()) - 1; i >= 0; i--) {
int height = (i + 1) * CFCHECKPT_INTERVAL;
block_index = block_index->GetAncestor(height);
Why this scored 23/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.