Merge #19259: fuzz: Add fuzzing harness for LoadMempool(...) and DumpMempool(...)

68afd3e tests: Add fuzzing harness for LoadMempool(...) and DumpMempool(...) (practicalswift)
91af6b9 validation: Make DumpMempool(...) and LoadMempool(...) easier to test/fuzz/mock (practicalswift)
af322c7 tests: Set errno in FuzzedFileProvider. Implement seek(..., ..., SEEK_END). (practicalswift)

Pull request description:

  Add fuzzing harness for `LoadMempool(...)` and `DumpMempool(...)`.

  See [`doc/fuzzing.md`](https://github.com/bitcoin/bitcoin/blob/master/doc/fuzzing.md) for information on how to fuzz Bitcoin Core. Don't forget to contribute any coverage increasing inputs you find to the [Bitcoin Core fuzzing corpus repo](https://github.com/bitcoin-core/qa-assets).

  Happy fuzzing :)

ACKs for top commit:
  jonatack:
    Tested re-ACK 68afd3e

Tree-SHA512: 4b5fcaa87e6eb478611d3b68eb6859645a5e121e7e3b056ad2815699dace0a6123706ff542def371b47f4ab3ce2b8a29782026d84fb505827121e9b4cc7dac31

Author: MarcoFalke

src/Makefile.test.include

@@ -297,7 +297,8 @@ test_fuzz_fuzz_SOURCES = \
  test/fuzz/transaction.cpp \
  test/fuzz/tx_in.cpp \
  test/fuzz/tx_out.cpp \
- test/fuzz/txrequest.cpp
+ test/fuzz/txrequest.cpp \
+ test/fuzz/validation_load_mempool.cpp
 endif # ENABLE_FUZZ_BINARY
 
 nodist_test_test_bitcoin_SOURCES = $(GENERATED_TEST_FILES)

src/test/fuzz/util.h

@@ -259,6 +259,16 @@ void SetFuzzedErrNo(FuzzedDataProvider& fuzzed_data_provider, const std::array<T
     errno = fuzzed_data_provider.PickValueInArray(errnos);
 }
 
+/*
+ * Sets a fuzzed errno in the range [0, 133 (EHWPOISON)]. Can be used from functions emulating
+ * standard library functions that set errno, or in other contexts where the value of errno
+ * might be relevant for the execution path that will be taken.
+ */
+inline void SetFuzzedErrNo(FuzzedDataProvider& fuzzed_data_provider) noexcept
+{
+    errno = fuzzed_data_provider.ConsumeIntegralInRange<int>(0, 133);
+}
+
 /**
  * Returns a byte vector of specified size regardless of the number of remaining bytes available
  * from the fuzzer. Pads with zero value bytes if needed to achieve the specified size.
@@ -345,6 +355,7 @@ class FuzzedFileProvider
 
     FILE* open()
     {
+        SetFuzzedErrNo(m_fuzzed_data_provider);
         if (m_fuzzed_data_provider.ConsumeBool()) {
             return nullptr;
         }
@@ -386,6 +397,7 @@ class FuzzedFileProvider
     static ssize_t read(void* cookie, char* buf, size_t size)
     {
         FuzzedFileProvider* fuzzed_file = (FuzzedFileProvider*)cookie;
+        SetFuzzedErrNo(fuzzed_file->m_fuzzed_data_provider);
         if (buf == nullptr || size == 0 || fuzzed_file->m_fuzzed_data_provider.ConsumeBool()) {
             return fuzzed_file->m_fuzzed_data_provider.ConsumeBool() ? 0 : -1;
         }
@@ -404,6 +416,7 @@ class FuzzedFileProvider
     static ssize_t write(void* cookie, const char* buf, size_t size)
     {
         FuzzedFileProvider* fuzzed_file = (FuzzedFileProvider*)cookie;
+        SetFuzzedErrNo(fuzzed_file->m_fuzzed_data_provider);
         const ssize_t n = fuzzed_file->m_fuzzed_data_provider.ConsumeIntegralInRange<ssize_t>(0, size);
         if (AdditionOverflow(fuzzed_file->m_offset, (int64_t)n)) {
             return fuzzed_file->m_fuzzed_data_provider.ConsumeBool() ? 0 : -1;
@@ -414,8 +427,9 @@ class FuzzedFileProvider
 
     static int seek(void* cookie, int64_t* offset, int whence)
     {
-        assert(whence == SEEK_SET || whence == SEEK_CUR); // SEEK_END not implemented yet.
+        assert(whence == SEEK_SET || whence == SEEK_CUR || whence == SEEK_END);
         FuzzedFileProvider* fuzzed_file = (FuzzedFileProvider*)cookie;
+        SetFuzzedErrNo(fuzzed_file->m_fuzzed_data_provider);
         int64_t new_offset = 0;
         if (whence == SEEK_SET) {
             new_offset = *offset;
@@ -424,6 +438,12 @@ class FuzzedFileProvider
                 return -1;
             }
             new_offset = fuzzed_file->m_offset + *offset;
+        } else if (whence == SEEK_END) {
+            const int64_t n = fuzzed_file->m_fuzzed_data_provider.ConsumeIntegralInRange<int64_t>(0, 4096);
+            if (AdditionOverflow(n, *offset)) {
+                return -1;
+            }
+            new_offset = n + *offset;
         }
         if (new_offset < 0) {
             return -1;
@@ -436,6 +456,7 @@ class FuzzedFileProvider
     static int close(void* cookie)
     {
         FuzzedFileProvider* fuzzed_file = (FuzzedFileProvider*)cookie;
+        SetFuzzedErrNo(fuzzed_file->m_fuzzed_data_provider);
         return fuzzed_file->m_fuzzed_data_provider.ConsumeIntegralInRange<int>(-1, 0);
     }
 };

src/test/fuzz/validation_load_mempool.cpp

@@ -0,0 +1,34 @@
+// Copyright (c) 2020 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#include <chainparamsbase.h>
+#include <test/fuzz/FuzzedDataProvider.h>
+#include <test/fuzz/fuzz.h>
+#include <test/fuzz/util.h>
+#include <test/util/setup_common.h>
+#include <txmempool.h>
+#include <util/time.h>
+#include <validation.h>
+
+#include <cstdint>
+#include <vector>
+
+void initialize_validation_load_mempool()
+{
+    static const auto testing_setup = MakeNoLogFileContext<const TestingSetup>();
+}
+
+FUZZ_TARGET_INIT(validation_load_mempool, initialize_validation_load_mempool)
+{
+    FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()};
+    SetMockTime(ConsumeTime(fuzzed_data_provider));
+    FuzzedFileProvider fuzzed_file_provider = ConsumeFile(fuzzed_data_provider);
+
+    CTxMemPool pool{};
+    auto fuzzed_fopen = [&](const fs::path&, const char*) {
+        return fuzzed_file_provider.open();
+    };
+    (void)LoadMempool(pool, ::ChainstateActive(), fuzzed_fopen);
+    (void)DumpMempool(pool, fuzzed_fopen, true);
+}

src/validation.cpp

@@ -5010,11 +5010,11 @@ CBlockFileInfo* GetBlockFileInfo(size_t n)
 
 static const uint64_t MEMPOOL_DUMP_VERSION = 1;
 
-bool LoadMempool(CTxMemPool& pool, CChainState& active_chainstate)
+bool LoadMempool(CTxMemPool& pool, CChainState& active_chainstate, FopenFn mockable_fopen_function)
 {
     const CChainParams& chainparams = Params();
     int64_t nExpiryTimeout = gArgs.GetArg("-mempoolexpiry", DEFAULT_MEMPOOL_EXPIRY) * 60 * 60;
-    FILE* filestr = fsbridge::fopen(GetDataDir() / "mempool.dat", "rb");
+    FILE* filestr{mockable_fopen_function(GetDataDir() / "mempool.dat", "rb")};
     CAutoFile file(filestr, SER_DISK, CLIENT_VERSION);
     if (file.IsNull()) {
         LogPrintf("Failed to open mempool file from disk. Continuing anyway.\n");
@@ -5095,7 +5095,7 @@ bool LoadMempool(CTxMemPool& pool, CChainState& active_chainstate)
     return true;
 }
 
-bool DumpMempool(const CTxMemPool& pool)
+bool DumpMempool(const CTxMemPool& pool, FopenFn mockable_fopen_function, bool skip_file_commit)
 {
     int64_t start = GetTimeMicros();
 
@@ -5118,7 +5118,7 @@ bool DumpMempool(const CTxMemPool& pool)
     int64_t mid = GetTimeMicros();
 
     try {
-        FILE* filestr = fsbridge::fopen(GetDataDir() / "mempool.dat.new", "wb");
+        FILE* filestr{mockable_fopen_function(GetDataDir() / "mempool.dat.new", "wb")};
         if (!filestr) {
             return false;
         }
@@ -5141,7 +5141,7 @@ bool DumpMempool(const CTxMemPool& pool)
         LogPrintf("Writing %d unbroadcast transactions to disk.\n", unbroadcast_txids.size());
         file << unbroadcast_txids;
 
-        if (!FileCommit(file.Get()))
+        if (!skip_file_commit && !FileCommit(file.Get()))
             throw std::runtime_error("FileCommit failed");
         file.fclose();
         if (!RenameOver(GetDataDir() / "mempool.dat.new", GetDataDir() / "mempool.dat")) {

src/validation.h

@@ -1013,11 +1013,13 @@ int32_t ComputeBlockVersion(const CBlockIndex* pindexPrev, const Consensus::Para
 /** Get block file info entry for one block file */
 CBlockFileInfo* GetBlockFileInfo(size_t n);
 
+using FopenFn = std::function<FILE*(const fs::path&, const char*)>;
+
 /** Dump the mempool to disk. */
-bool DumpMempool(const CTxMemPool& pool);
+bool DumpMempool(const CTxMemPool& pool, FopenFn mockable_fopen_function = fsbridge::fopen, bool skip_file_commit = false);
 
 /** Load the mempool from disk. */
-bool LoadMempool(CTxMemPool& pool, CChainState& active_chainstate);
+bool LoadMempool(CTxMemPool& pool, CChainState& active_chainstate, FopenFn mockable_fopen_function = fsbridge::fopen);
 
 //! Check whether the block associated with this index entry is pruned or not.
 inline bool IsBlockPruned(const CBlockIndex* pblockindex)