diff options
author | MarcoFalke <falke.marco@gmail.com> | 2021-06-28 16:11:41 +0200 |
---|---|---|
committer | MarcoFalke <falke.marco@gmail.com> | 2021-06-28 16:11:44 +0200 |
commit | 3f56ef7bef22f0c8c94ad7e401d50b188dae2cbe (patch) | |
tree | 26ef1ec0de73a261f86b55f6dd82328ad0689a62 | |
parent | 8cdf91735f2bdc55577d84a9915f5920ce23b00a (diff) | |
parent | fa9ebedec3f982bb5bb459ea33d74c94d9b5cec4 (diff) | |
download | bitcoin-3f56ef7bef22f0c8c94ad7e401d50b188dae2cbe.tar.xz |
Merge bitcoin/bitcoin#22146: Reject invalid coin height and output index when loading assumeutxo
fa9ebedec3f982bb5bb459ea33d74c94d9b5cec4 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 fa9ebedec3f982bb5bb459ea33d74c94d9b5cec4: patch looks correct
jamesob:
crACK https://github.com/bitcoin/bitcoin/pull/22146/commits/fa9ebedec3f982bb5bb459ea33d74c94d9b5cec4
theStack:
Code review ACK fa9ebedec3f982bb5bb459ea33d74c94d9b5cec4
benthecarman:
crACK fa9ebedec3f982bb5bb459ea33d74c94d9b5cec4
Tree-SHA512: dae7caee4b3862b23ebdf2acb7edec4baf75b0dbf1409b370b1a73aa6b632b317ebfac596dcbaf4edfb1301b513f45465ea75328962460f35e2af0d7e547c9ac
-rw-r--r-- | src/validation.cpp | 8 | ||||
-rw-r--r-- | test/sanitizer_suppressions/ubsan | 3 |
2 files changed, 8 insertions, 3 deletions
diff --git a/src/validation.cpp b/src/validation.cpp index b48e49a10b..fe4de0d218 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -4879,6 +4879,14 @@ bool ChainstateManager::PopulateAndValidateSnapshot( coins_count - coins_left); return false; } + if (coin.nHeight > base_height || + outpoint.n >= std::numeric_limits<decltype(outpoint.n)>::max() // Avoid integer wrap-around in coinstats.cpp:ApplyHash + ) { + LogPrintf("[snapshot] bad snapshot data after deserializing %d coins\n", + coins_count - coins_left); + return false; + } + coins_cache.EmplaceCoinInternalDANGER(std::move(outpoint), std::move(coin)); --coins_left; diff --git a/test/sanitizer_suppressions/ubsan b/test/sanitizer_suppressions/ubsan index 877adaccec..2850cfcea5 100644 --- a/test/sanitizer_suppressions/ubsan +++ b/test/sanitizer_suppressions/ubsan @@ -34,9 +34,6 @@ unsigned-integer-overflow:crypto/ unsigned-integer-overflow:FuzzedDataProvider.h unsigned-integer-overflow:hash.cpp unsigned-integer-overflow:leveldb/ -# temporary coinstats suppressions (will be removed and fixed in https://github.com/bitcoin/bitcoin/pull/22146) -unsigned-integer-overflow:node/coinstats.cpp -signed-integer-overflow:node/coinstats.cpp unsigned-integer-overflow:policy/fees.cpp unsigned-integer-overflow:prevector.h unsigned-integer-overflow:pubkey.h |