From 56d878c4650cc46ce54d1d79db054ac44b486605 Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Tue, 2 Jan 2024 13:24:02 +0100 Subject: [PATCH 1/3] fuzz: avoid underflow in coins_view target --- src/test/fuzz/coins_view.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/test/fuzz/coins_view.cpp b/src/test/fuzz/coins_view.cpp index 9c6aa6e7a1e..e6361098314 100644 --- a/src/test/fuzz/coins_view.cpp +++ b/src/test/fuzz/coins_view.cpp @@ -69,6 +69,12 @@ FUZZ_TARGET(coins_view, .init = initialize_coins_view) if (e.what() == std::string{"Attempted to overwrite an unspent coin (when possible_overwrite is false)"}) { assert(!possible_overwrite); expected_code_path = true; + // AddCoin() decreases cachedCoinsUsage by the memory usage of the old coin at the beginning and + // increases it by the value of the new coin at the end. If it throws in the process, the value + // of cachedCoinsUsage would have been incorrectly decreased, leading to an underflow later on. + // To avoid this, use Flush() to reset the value of cachedCoinsUsage in sync with the cacheCoins + // mapping. + (void)coins_view_cache.Flush(); } } assert(expected_code_path); From 46e14630f7fe8e2d51adc18a4f50345eb26970c7 Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Fri, 4 Aug 2023 11:54:02 +0200 Subject: [PATCH 2/3] fuzz: move the coins_view target's body into a standalone function We'll reuse it for a target where the coins view is a DB. --- src/test/fuzz/coins_view.cpp | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/test/fuzz/coins_view.cpp b/src/test/fuzz/coins_view.cpp index e6361098314..396e4f17e70 100644 --- a/src/test/fuzz/coins_view.cpp +++ b/src/test/fuzz/coins_view.cpp @@ -14,6 +14,7 @@ #include #include #include +#include #include #include @@ -41,12 +42,10 @@ void initialize_coins_view() static const auto testing_setup = MakeNoLogFileContext<>(); } -FUZZ_TARGET(coins_view, .init = initialize_coins_view) +void TestCoinsView(FuzzedDataProvider& fuzzed_data_provider, CCoinsView& backend_coins_view) { - FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()}; bool good_data{true}; - CCoinsView backend_coins_view; CCoinsViewCache coins_view_cache{&backend_coins_view, /*deterministic=*/true}; COutPoint random_out_point; Coin random_coin; @@ -294,3 +293,10 @@ FUZZ_TARGET(coins_view, .init = initialize_coins_view) }); } } + +FUZZ_TARGET(coins_view, .init = initialize_coins_view) +{ + FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()}; + CCoinsView backend_coins_view; + TestCoinsView(fuzzed_data_provider, backend_coins_view); +} From cfc42ae5b7ef6d3892907cfe36dc3ae923495d37 Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Fri, 4 Aug 2023 13:49:30 +0200 Subject: [PATCH 3/3] fuzz: add a target for the coins database It reuses the logic from the `coins_view` target, except it uses an in-memory CCoinsViewDB as the backend. Note `CCoinsViewDB` will assert the best block hash is never null, so we slightly modify the coins_view fuzz logic to take care of this. --- src/test/fuzz/coins_view.cpp | 32 ++++++++++++++++++++++++++------ 1 file changed, 26 insertions(+), 6 deletions(-) diff --git a/src/test/fuzz/coins_view.cpp b/src/test/fuzz/coins_view.cpp index 396e4f17e70..2b9b81e0306 100644 --- a/src/test/fuzz/coins_view.cpp +++ b/src/test/fuzz/coins_view.cpp @@ -1,4 +1,4 @@ -// Copyright (c) 2020-2022 The Bitcoin Core developers +// Copyright (c) 2020-present The Bitcoin Core developers // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. @@ -42,11 +42,12 @@ void initialize_coins_view() static const auto testing_setup = MakeNoLogFileContext<>(); } -void TestCoinsView(FuzzedDataProvider& fuzzed_data_provider, CCoinsView& backend_coins_view) +void TestCoinsView(FuzzedDataProvider& fuzzed_data_provider, CCoinsView& backend_coins_view, bool is_db) { bool good_data{true}; CCoinsViewCache coins_view_cache{&backend_coins_view, /*deterministic=*/true}; + if (is_db) coins_view_cache.SetBestBlock(uint256::ONE); COutPoint random_out_point; Coin random_coin; CMutableTransaction random_mutable_transaction; @@ -85,7 +86,10 @@ void TestCoinsView(FuzzedDataProvider& fuzzed_data_provider, CCoinsView& backend (void)coins_view_cache.Sync(); }, [&] { - coins_view_cache.SetBestBlock(ConsumeUInt256(fuzzed_data_provider)); + uint256 best_block{ConsumeUInt256(fuzzed_data_provider)}; + // Set best block hash to non-null to satisfy the assertion in CCoinsViewDB::BatchWrite(). + if (is_db && best_block.IsNull()) best_block = uint256::ONE; + coins_view_cache.SetBestBlock(best_block); }, [&] { Coin move_to; @@ -153,7 +157,11 @@ void TestCoinsView(FuzzedDataProvider& fuzzed_data_provider, CCoinsView& backend bool expected_code_path = false; try { auto cursor{CoinsViewCacheCursor(usage, sentinel, coins_map, /*will_erase=*/true)}; - coins_view_cache.BatchWrite(cursor, fuzzed_data_provider.ConsumeBool() ? ConsumeUInt256(fuzzed_data_provider) : coins_view_cache.GetBestBlock()); + uint256 best_block{coins_view_cache.GetBestBlock()}; + if (fuzzed_data_provider.ConsumeBool()) best_block = ConsumeUInt256(fuzzed_data_provider); + // Set best block hash to non-null to satisfy the assertion in CCoinsViewDB::BatchWrite(). + if (is_db && best_block.IsNull()) best_block = uint256::ONE; + coins_view_cache.BatchWrite(cursor, best_block); expected_code_path = true; } catch (const std::logic_error& e) { if (e.what() == std::string{"FRESH flag misapplied to coin that exists in parent cache"}) { @@ -207,7 +215,7 @@ void TestCoinsView(FuzzedDataProvider& fuzzed_data_provider, CCoinsView& backend { std::unique_ptr coins_view_cursor = backend_coins_view.Cursor(); - assert(!coins_view_cursor); + assert(is_db == !!coins_view_cursor); (void)backend_coins_view.EstimateSize(); (void)backend_coins_view.GetBestBlock(); (void)backend_coins_view.GetHeadBlocks(); @@ -298,5 +306,17 @@ FUZZ_TARGET(coins_view, .init = initialize_coins_view) { FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()}; CCoinsView backend_coins_view; - TestCoinsView(fuzzed_data_provider, backend_coins_view); + TestCoinsView(fuzzed_data_provider, backend_coins_view, /*is_db=*/false); +} + +FUZZ_TARGET(coins_view_db, .init = initialize_coins_view) +{ + FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()}; + auto db_params = DBParams{ + .path = "", + .cache_bytes = 1_MiB, + .memory_only = true, + }; + CCoinsViewDB coins_db{std::move(db_params), CoinsViewOptions{}}; + TestCoinsView(fuzzed_data_provider, coins_db, /*is_db=*/true); }