Skip to content

Commit 37af8bf

Browse files
committed
Merge bitcoin/bitcoin#31549: fuzz: Abort if system time is called without mock time being set
a96b84c fuzz: Abort when calling system time without setting mock time (marcofleon) ff21870 fuzz: Add SetMockTime() to necessary targets (marcofleon) Pull request description: This PR expands the `CheckGlobals` utility that was introduced in bitcoin/bitcoin#31486 and should help with fuzz stability (bitcoin/bitcoin#29018). System time shouldn't be used when running a fuzz test, as it is likely to introduce instability (non-determinism). This PR identifies and fixes the targets that were calling system time without setting mock time at the start of an iteration. Removing`SetMockTime()` from any one of these targets should result in a crash and a message describing the issue. ACKs for top commit: achow101: ACK a96b84c dergoegge: Code review ACK a96b84c brunoerg: crACK a96b84c Tree-SHA512: e093a9feb8a397954f7b1416dfa8790b2733f09d5ac51fda5a9d225a55ebd8f99135aa52bdf5ab531653ad1a3739c4ca2b5349c1d989bb4b009ec8eaad684f7d
2 parents 54115d8 + a96b84c commit 37af8bf

17 files changed

+65
-5
lines changed

src/test/fuzz/fuzz.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,9 @@ void initialize()
116116
// - Creating a BasicTestingSetup or derived class will switch to a random seed.
117117
SeedRandomStateForTest(SeedRand::ZEROS);
118118

119+
// Set time to the genesis block timestamp for deterministic initialization.
120+
SetMockTime(1231006505);
121+
119122
// Terminate immediately if a fuzzing harness ever tries to create a socket.
120123
// Individual tests can override this by pointing CreateSock to a mocked alternative.
121124
CreateSock = [](int, int, int) -> std::unique_ptr<Sock> { std::terminate(); };

src/test/fuzz/load_external_block_file.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
#include <test/fuzz/fuzz.h>
1010
#include <test/fuzz/util.h>
1111
#include <test/util/setup_common.h>
12+
#include <util/time.h>
1213
#include <validation.h>
1314

1415
#include <cstdint>
@@ -27,6 +28,7 @@ void initialize_load_external_block_file()
2728
FUZZ_TARGET(load_external_block_file, .init = initialize_load_external_block_file)
2829
{
2930
FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()};
31+
SetMockTime(ConsumeTime(fuzzed_data_provider));
3032
FuzzedFileProvider fuzzed_file_provider{fuzzed_data_provider};
3133
AutoFile fuzzed_block_file{fuzzed_file_provider.open()};
3234
if (fuzzed_block_file.IsNull()) {

src/test/fuzz/mini_miner.cpp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,7 @@
1+
// Copyright (c) The Bitcoin Core developers
2+
// Distributed under the MIT software license, see the accompanying
3+
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
4+
15
#include <test/fuzz/FuzzedDataProvider.h>
26
#include <test/fuzz/fuzz.h>
37
#include <test/fuzz/util.h>
@@ -13,6 +17,7 @@
1317
#include <random.h>
1418
#include <txmempool.h>
1519
#include <util/check.h>
20+
#include <util/time.h>
1621
#include <util/translation.h>
1722

1823
#include <deque>
@@ -36,6 +41,7 @@ FUZZ_TARGET(mini_miner, .init = initialize_miner)
3641
{
3742
SeedRandomStateForTest(SeedRand::ZEROS);
3843
FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()};
44+
SetMockTime(ConsumeTime(fuzzed_data_provider));
3945
bilingual_str error;
4046
CTxMemPool pool{CTxMemPool::Options{}, error};
4147
Assert(error.empty());
@@ -115,6 +121,7 @@ FUZZ_TARGET(mini_miner_selection, .init = initialize_miner)
115121
{
116122
SeedRandomStateForTest(SeedRand::ZEROS);
117123
FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()};
124+
SetMockTime(ConsumeTime(fuzzed_data_provider));
118125
bilingual_str error;
119126
CTxMemPool pool{CTxMemPool::Options{}, error};
120127
Assert(error.empty());

src/test/fuzz/net.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
#include <test/util/setup_common.h>
1717
#include <util/asmap.h>
1818
#include <util/chaintype.h>
19+
#include <util/time.h>
1920

2021
#include <cstdint>
2122
#include <optional>
@@ -79,6 +80,7 @@ FUZZ_TARGET(net, .init = initialize_net)
7980
FUZZ_TARGET(local_address, .init = initialize_net)
8081
{
8182
FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size());
83+
SetMockTime(ConsumeTime(fuzzed_data_provider));
8284
CService service{ConsumeService(fuzzed_data_provider)};
8385
CNode node{ConsumeNode(fuzzed_data_provider)};
8486
{

src/test/fuzz/p2p_headers_presync.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -154,14 +154,15 @@ void initialize()
154154
FUZZ_TARGET(p2p_headers_presync, .init = initialize)
155155
{
156156
SeedRandomStateForTest(SeedRand::ZEROS);
157+
FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()};
158+
SetMockTime(ConsumeTime(fuzzed_data_provider));
159+
157160
ChainstateManager& chainman = *g_testing_setup->m_node.chainman;
158161

159162
LOCK(NetEventsInterface::g_msgproc_mutex);
160163

161164
g_testing_setup->ResetAndInitialize();
162165

163-
FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()};
164-
165166
CBlockHeader base{chainman.GetParams().GenesisBlock()};
166167
SetMockTime(base.nTime);
167168

src/test/fuzz/partially_downloaded_block.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
#include <test/util/txmempool.h>
1212
#include <txmempool.h>
1313
#include <util/check.h>
14+
#include <util/time.h>
1415
#include <util/translation.h>
1516

1617
#include <cstddef>
@@ -46,6 +47,7 @@ FUZZ_TARGET(partially_downloaded_block, .init = initialize_pdb)
4647
{
4748
SeedRandomStateForTest(SeedRand::ZEROS);
4849
FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()};
50+
SetMockTime(ConsumeTime(fuzzed_data_provider));
4951

5052
auto block{ConsumeDeserializable<CBlock>(fuzzed_data_provider, TX_WITH_WITNESS)};
5153
if (!block || block->vtx.size() == 0 ||

src/test/fuzz/socks5.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
#include <test/fuzz/util.h>
1010
#include <test/fuzz/util/net.h>
1111
#include <test/util/setup_common.h>
12+
#include <util/time.h>
1213

1314
#include <cstdint>
1415
#include <string>
@@ -29,6 +30,7 @@ void initialize_socks5()
2930
FUZZ_TARGET(socks5, .init = initialize_socks5)
3031
{
3132
FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()};
33+
SetMockTime(ConsumeTime(fuzzed_data_provider));
3234
ProxyCredentials proxy_credentials;
3335
proxy_credentials.username = fuzzed_data_provider.ConsumeRandomLengthString(512);
3436
proxy_credentials.password = fuzzed_data_provider.ConsumeRandomLengthString(512);

src/test/fuzz/txdownloadman.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
#include <test/util/txmempool.h>
1919
#include <util/hasher.h>
2020
#include <util/rbf.h>
21+
#include <util/time.h>
2122
#include <txmempool.h>
2223
#include <validation.h>
2324
#include <validationinterface.h>
@@ -167,6 +168,7 @@ FUZZ_TARGET(txdownloadman, .init = initialize)
167168
{
168169
SeedRandomStateForTest(SeedRand::ZEROS);
169170
FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size());
171+
SetMockTime(ConsumeTime(fuzzed_data_provider));
170172

171173
// Initialize txdownloadman
172174
bilingual_str error;
@@ -297,6 +299,7 @@ FUZZ_TARGET(txdownloadman_impl, .init = initialize)
297299
{
298300
SeedRandomStateForTest(SeedRand::ZEROS);
299301
FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size());
302+
SetMockTime(ConsumeTime(fuzzed_data_provider));
300303

301304
// Initialize a TxDownloadManagerImpl
302305
bilingual_str error;

src/test/fuzz/util/check_globals.cpp

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
#include <test/fuzz/util/check_globals.h>
66

77
#include <test/util/random.h>
8+
#include <util/time.h>
89

910
#include <iostream>
1011
#include <memory>
@@ -16,6 +17,8 @@ struct CheckGlobalsImpl {
1617
{
1718
g_used_g_prng = false;
1819
g_seeded_g_prng_zero = false;
20+
g_used_system_time = false;
21+
SetMockTime(0s);
1922
}
2023
~CheckGlobalsImpl()
2124
{
@@ -34,6 +37,19 @@ struct CheckGlobalsImpl {
3437
<< std::endl;
3538
std::abort(); // Abort, because AFL may try to recover from a std::exit
3639
}
40+
41+
if (g_used_system_time) {
42+
std::cerr << "\n\n"
43+
"The current fuzz target accessed system time.\n\n"
44+
45+
"This is acceptable, but requires the fuzz target to call \n"
46+
"SetMockTime() at the beginning of processing the fuzz input.\n\n"
47+
48+
"Without setting mock time, time-dependent behavior can lead \n"
49+
"to non-reproducible bugs or inefficient fuzzing.\n\n"
50+
<< std::endl;
51+
std::abort();
52+
}
3753
}
3854
};
3955

src/test/fuzz/util/check_globals.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,13 @@
55
#ifndef BITCOIN_TEST_FUZZ_UTIL_CHECK_GLOBALS_H
66
#define BITCOIN_TEST_FUZZ_UTIL_CHECK_GLOBALS_H
77

8+
#include <atomic>
89
#include <memory>
910
#include <optional>
1011
#include <string>
1112

13+
extern std::atomic<bool> g_used_system_time;
14+
1215
struct CheckGlobalsImpl;
1316
struct CheckGlobals {
1417
CheckGlobals();

0 commit comments

Comments
 (0)