Skip to content

Commit

Permalink
Merge bitcoin/bitcoin#31548: fuzz: Abort when global PRNG is used bef…
Browse files Browse the repository at this point in the history
…ore SeedRand::ZEROS

fa3c787 fuzz: Abort when global PRNG is used before SeedRand::ZEROS (MarcoFalke)

Pull request description:

  This adds one more check to abort when global PRNG is used before SeedRand::ZEROS in fuzz tests. This is achieved by carving out the two remaining uses. First, `g_rng_temp_path_init`, and second the random fallback for `RANDOM_CTX_SEED`, which isn't used in fuzz tests anyway.

  Requested in bitcoin/bitcoin#31521 (comment)

  Can be tested by reverting fadd568 and observing an abort when running the `utxo_total_supply` fuzz target.

ACKs for top commit:
  marcofleon:
    ACK fa3c787
  hodlinator:
    re-ACK fa3c787
  ryanofsky:
    Code review ACK fa3c787. This adds a new check to make that sure that RNG is never seeded during fuzzing after the RNG has been used. Together with existing checks which ensure RNG can only be seeded with zeroes during fuzzing, and that RNG must was seeded at some point if used after fuzzing, this implies it must have been seeded by zeros before being used.

Tree-SHA512: 2614928d31c310309bd9021b3e5637b35f64196020fbf9409e978628799691d0efd3f4cf606be9a2db0ef60b010f890c2e70c910eaa2934a7fbf64cd1598fe22
  • Loading branch information
ryanofsky committed Jan 22, 2025
2 parents 5d6f6fd + fa3c787 commit 78fa88c
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 6 deletions.
6 changes: 3 additions & 3 deletions src/test/fuzz/fuzz.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ void FuzzFrameworkRegisterTarget(std::string_view name, TypeTestOneInput target,
static std::string_view g_fuzz_target;
static const TypeTestOneInput* g_test_one_input{nullptr};

inline void test_one_input(FuzzBufferType buffer)
static void test_one_input(FuzzBufferType buffer)
{
CheckGlobals check{};
(*Assert(g_test_one_input))(buffer);
Expand Down Expand Up @@ -108,12 +108,12 @@ void ResetCoverageCounters() {}
#endif


void initialize()
static void initialize()
{
// By default, make the RNG deterministic with a fixed seed. This will affect all
// randomness during the fuzz test, except:
// - GetStrongRandBytes(), which is used for the creation of private key material.
// - Creating a BasicTestingSetup or derived class will switch to a random seed.
// - Randomness obtained before this call in g_rng_temp_path_init
SeedRandomStateForTest(SeedRand::ZEROS);

// Set time to the genesis block timestamp for deterministic initialization.
Expand Down
8 changes: 5 additions & 3 deletions src/test/util/random.cpp
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) 2023 The Bitcoin Core developers
// Copyright (c) 2023-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.

Expand All @@ -24,7 +24,8 @@ void SeedRandomStateForTest(SeedRand seedtype)
// MakeRandDeterministicDANGEROUS is called, the output of GetRandHash is
// no longer truly random. It should be enough to get the seed once for the
// process.
static const uint256 ctx_seed = []() {
static const auto g_ctx_seed = []() -> std::optional<uint256> {
if constexpr (G_FUZZING) return {};
// If RANDOM_CTX_SEED is set, use that as seed.
if (const char* num{std::getenv(RANDOM_CTX_SEED)}) {
if (auto num_parsed{uint256::FromUserHex(num)}) {
Expand All @@ -41,8 +42,9 @@ void SeedRandomStateForTest(SeedRand seedtype)
g_seeded_g_prng_zero = seedtype == SeedRand::ZEROS;
if constexpr (G_FUZZING) {
Assert(g_seeded_g_prng_zero); // Only SeedRandomStateForTest(SeedRand::ZEROS) is allowed in fuzz tests
Assert(!g_used_g_prng); // The global PRNG must not have been used before SeedRandomStateForTest(SeedRand::ZEROS)
}
const uint256& seed{seedtype == SeedRand::FIXED_SEED ? ctx_seed : uint256::ZERO};
const uint256& seed{seedtype == SeedRand::FIXED_SEED ? g_ctx_seed.value() : uint256::ZERO};
LogInfo("Setting random seed for current tests to %s=%s\n", RANDOM_CTX_SEED, seed.GetHex());
MakeRandDeterministicDANGEROUS(seed);
}
2 changes: 2 additions & 0 deletions src/test/util/setup_common.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,9 @@ constexpr inline auto TEST_DIR_PATH_ELEMENT{"test_common bitcoin"}; // Includes
static FastRandomContext g_rng_temp_path;
static const bool g_rng_temp_path_init{[] {
// Must be initialized before any SeedRandomForTest
Assert(!g_used_g_prng);
(void)g_rng_temp_path.rand64();
g_used_g_prng = false;
return true;
}()};

Expand Down

0 comments on commit 78fa88c

Please sign in to comment.