Skip to content

Commit 3f56ef7

Browse files
author
MarcoFalke
committed
Merge bitcoin/bitcoin#22146: Reject invalid coin height and output index when loading assumeutxo
fa9ebed Reject invalid coin height and output index when loading assumeutxo (MarcoFalke) Pull request description: It should be impossible to have a coin at a height higher than the height of the snapshot block, so reject those early to avoid integer wraparounds and hash collisions later on. Same for the outpoint index. Both issues were found by fuzzing: * The height issue by OSS-Fuzz: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=34793 * The outpoint issue by my fuzz server: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=34793#c2 ACKs for top commit: practicalswift: cr ACK fa9ebed: patch looks correct jamesob: crACK bitcoin/bitcoin@fa9ebed theStack: Code review ACK fa9ebed benthecarman: crACK fa9ebed Tree-SHA512: dae7caee4b3862b23ebdf2acb7edec4baf75b0dbf1409b370b1a73aa6b632b317ebfac596dcbaf4edfb1301b513f45465ea75328962460f35e2af0d7e547c9ac
2 parents 8cdf917 + fa9ebed commit 3f56ef7

File tree

2 files changed

+8
-3
lines changed

2 files changed

+8
-3
lines changed

src/validation.cpp

+8
Original file line numberDiff line numberDiff line change
@@ -4879,6 +4879,14 @@ bool ChainstateManager::PopulateAndValidateSnapshot(
48794879
coins_count - coins_left);
48804880
return false;
48814881
}
4882+
if (coin.nHeight > base_height ||
4883+
outpoint.n >= std::numeric_limits<decltype(outpoint.n)>::max() // Avoid integer wrap-around in coinstats.cpp:ApplyHash
4884+
) {
4885+
LogPrintf("[snapshot] bad snapshot data after deserializing %d coins\n",
4886+
coins_count - coins_left);
4887+
return false;
4888+
}
4889+
48824890
coins_cache.EmplaceCoinInternalDANGER(std::move(outpoint), std::move(coin));
48834891

48844892
--coins_left;

test/sanitizer_suppressions/ubsan

-3
Original file line numberDiff line numberDiff line change
@@ -34,9 +34,6 @@ unsigned-integer-overflow:crypto/
3434
unsigned-integer-overflow:FuzzedDataProvider.h
3535
unsigned-integer-overflow:hash.cpp
3636
unsigned-integer-overflow:leveldb/
37-
# temporary coinstats suppressions (will be removed and fixed in https://github.com/bitcoin/bitcoin/pull/22146)
38-
unsigned-integer-overflow:node/coinstats.cpp
39-
signed-integer-overflow:node/coinstats.cpp
4037
unsigned-integer-overflow:policy/fees.cpp
4138
unsigned-integer-overflow:prevector.h
4239
unsigned-integer-overflow:pubkey.h

0 commit comments

Comments
 (0)