Merge pull request #5326 from PastaPastaPasta/develop-trivial-2023-04-15

backport: trivial 2023 04 15
2024-12-25 03:52:49 +01:00 · 2023-04-17 00:57:54 +03:00 · 2023-04-17 00:57:54 +03:00 · 47e45810a9
commit 47e45810a9
parent a4d894943a 8ca3005a91
18 changed files with 80 additions and 33 deletions
--- a/Makefile.am
+++ b/Makefile.am
@ -4,7 +4,7 @@
 # file COPYING or http://www.opensource.org/licenses/mit-license.php.

 # Pattern rule to print variables, e.g. make print-top_srcdir
-print-%:
+print-%: FORCE
 	@echo '$*'='$($*)'

 ACLOCAL_AMFLAGS = -I build-aux/m4
--- a/contrib/devtools/test-symbol-check.py
+++ b/contrib/devtools/test-symbol-check.py
@ -67,20 +67,21 @@ class TestSymbolChecks(unittest.TestCase):
                (1, executable + ': NEEDED library libutil.so.1 is not allowed\n' +
                    executable + ': failed LIBRARY_DEPENDENCIES'))

-        # finally, check a conforming file that simply uses a math function
+        # finally, check a simple conforming binary
        source = 'test3.c'
        executable = 'test3'
        with open(source, 'w', encoding="utf8") as f:
            f.write('''
-                #include <math.h>
+                #include <stdio.h>

                int main()
                {
-                    return (int)pow(2.0, 4.0);
+                    printf("42");
+                    return 0;
                }
        ''')

-        self.assertEqual(call_symbol_check(cc, source, executable, ['-lm']),
+        self.assertEqual(call_symbol_check(cc, source, executable, []),
                (0, ''))

    def test_MACHO(self):
--- a/depends/Makefile
+++ b/depends/Makefile
@ -1,7 +1,7 @@
 .NOTPARALLEL :

 # Pattern rule to print variables, e.g. make print-top_srcdir
-print-%:
+print-%: FORCE
 	@echo '$*'='$($*)'

 # When invoking a sub-make, keep only the command line variable definitions
@ -270,3 +270,4 @@ download: download-osx download-linux download-win
 $(foreach package,$(all_packages),$(eval $(call ext_add_stages,$(package))))

 .PHONY: install cached clean clean-all download-one download-osx download-linux download-win download check-packages check-sources
+.PHONY: FORCE
--- a/doc/fuzzing.md
+++ b/doc/fuzzing.md
@ -16,7 +16,7 @@ $ FUZZ=process_message src/test/fuzz/fuzz
 # abort fuzzing using ctrl-c
 ```

-## Fuzzing harnesses, fuzzing output and fuzzing corpora
+## Fuzzing harnesses and output

 [`process_message`](https://github.com/dashpay/dash/blob/develop/src/test/fuzz/process_message.cpp) is a fuzzing harness for the [`ProcessMessage(...)` function (`net_processing`)](https://github.com/dashpay/dash/blob/develop/src/net_processing.cpp). The available fuzzing harnesses are found in [`src/test/fuzz/`](https://github.com/dashpay/dash/tree/develop/src/test/fuzz).

@ -64,6 +64,8 @@ block^@M-^?M-^?M-^?M-^?M-^?nM-^?M-^?

 In this case the fuzzer managed to create a `block` message which when passed to `ProcessMessage(...)` increased coverage.

+## Fuzzing corpora
+
 The project's collection of seed corpora is found in the [`bitcoin-core/qa-assets`](https://github.com/bitcoin-core/qa-assets) repo.

 To fuzz `process_message` using the [`bitcoin-core/qa-assets`](https://github.com/bitcoin-core/qa-assets) seed corpus:
@ -81,6 +83,20 @@ INFO: seed corpus: files: 991 min: 1b max: 1858b total: 288291b rss: 150Mb
 …
 ```

+## Reproduce a fuzzer crash reported by the CI
+
+- `cd` into the `qa-assets` directory and update it with `git pull qa-assets`
+- locate the crash case described in the CI output, e.g. `Test unit written to
+  ./crash-1bc91feec9fc00b107d97dc225a9f2cdaa078eb6`
+- make sure to compile with all sanitizers, if they are needed (fuzzing runs
+  more slowly with sanitizers enabled, but a crash should be reproducible very
+  quickly from a crash case)
+- run the fuzzer with the case number appended to the seed corpus path:
+  `FUZZ=process_message src/test/fuzz/fuzz
+  qa-assets/fuzz_seed_corpus/process_message/1bc91feec9fc00b107d97dc225a9f2cdaa078eb6`
+
+## Submit improved coverage
+
 If you find coverage increasing inputs when fuzzing you are highly encouraged to submit them for inclusion in the [`bitcoin-core/qa-assets`](https://github.com/bitcoin-core/qa-assets) repo.

 Every single pull request submitted against the Dash Core repo is automatically tested against all inputs in the [`bitcoin-core/qa-assets`](https://github.com/bitcoin-core/qa-assets) repo. Contributing new coverage increasing inputs is an easy way to help make Dash Core more robust.
--- a/src/Makefile.am
+++ b/src/Makefile.am
@ -4,7 +4,7 @@
 # file COPYING or http://www.opensource.org/licenses/mit-license.php.

 # Pattern rule to print variables, e.g. make print-top_srcdir
-print-%:
+print-%: FORCE
 	@echo '$*'='$($*)'

 DIST_SUBDIRS = secp256k1
--- a/src/interfaces/chain.h
+++ b/src/interfaces/chain.h
@ -46,7 +46,9 @@ namespace interfaces {
 class Wallet;
 class Handler;

-//! Helper for findBlock to selectively return pieces of block data.
+//! Helper for findBlock to selectively return pieces of block data. If block is
+//! found, data will be returned by setting specified output variables. If block
+//! is not found, output variables will keep their previous values.
 class FoundBlock
 {
 public:
@ -71,6 +73,7 @@ public:
    bool* m_in_active_chain = nullptr;
    const FoundBlock* m_next_block = nullptr;
    CBlock* m_data = nullptr;
+    mutable bool found = false;
 };

 //! Interface giving clients (wallet processes, maybe other analysis tools in
--- a/src/node/interfaces.cpp
+++ b/src/node/interfaces.cpp
@ -483,6 +483,7 @@ bool FillBlock(const CBlockIndex* index, const FoundBlock& block, UniqueLock<Rec
        REVERSE_LOCK(lock);
        if (!ReadBlockFromDisk(*block.m_data, index, Params().GetConsensus())) block.m_data->SetNull();
    }
+    block.found = true;
    return true;
 }

--- a/src/test/fuzz/addrman.cpp
+++ b/src/test/fuzz/addrman.cpp
@ -34,6 +34,17 @@ FUZZ_TARGET_INIT(addrman, initialize_addrman)
            addr_man.m_asmap.clear();
        }
    }
+    if (fuzzed_data_provider.ConsumeBool()) {
+        const std::vector<uint8_t> serialized_data{ConsumeRandomLengthByteVector(fuzzed_data_provider)};
+        CDataStream ds(serialized_data, SER_DISK, INIT_PROTO_VERSION);
+        const auto ser_version{fuzzed_data_provider.ConsumeIntegral<int32_t>()};
+        ds.SetVersion(ser_version);
+        try {
+            ds >> addr_man;
+        } catch (const std::ios_base::failure&) {
+            addr_man.Clear();
+        }
+    }
    while (fuzzed_data_provider.ConsumeBool()) {
        CallOneOf(
            fuzzed_data_provider,
--- a/src/test/fuzz/banman.cpp
+++ b/src/test/fuzz/banman.cpp
@ -31,12 +31,16 @@ void initialize_banman()

 FUZZ_TARGET_INIT(banman, initialize_banman)
 {
+    // The complexity is O(N^2), where N is the input size, because each call
+    // might call DumpBanlist (or other methods that are at least linear
+    // complexity of the input size).
+    int limit_max_ops{300};
    FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()};
    const fs::path banlist_file = GetDataDir() / "fuzzed_banlist.dat";
    fs::remove(banlist_file);
    {
        BanMan ban_man{banlist_file, nullptr, ConsumeBanTimeOffset(fuzzed_data_provider)};
-        while (fuzzed_data_provider.ConsumeBool()) {
+        while (--limit_max_ops >= 0 && fuzzed_data_provider.ConsumeBool()) {
            CallOneOf(
                fuzzed_data_provider,
                [&] {
@ -50,7 +54,6 @@ FUZZ_TARGET_INIT(banman, initialize_banman)
                [&] {
                    ban_man.ClearBanned();
                },
-                [] {},
                [&] {
                    ban_man.IsBanned(ConsumeNetAddr(fuzzed_data_provider));
                },
@ -70,7 +73,6 @@ FUZZ_TARGET_INIT(banman, initialize_banman)
                [&] {
                    ban_man.DumpBanlist();
                },
-                [] {},
                [&] {
                    ban_man.Discourage(ConsumeNetAddr(fuzzed_data_provider));
                });
--- a/src/test/fuzz/base_encode_decode.cpp
+++ b/src/test/fuzz/base_encode_decode.cpp
@ -15,7 +15,12 @@
 #include <string>
 #include <vector>

-FUZZ_TARGET(base_encode_decode)
+void initialize_base_encode_decode()
+{
+    static const ECCVerifyHandle verify_handle;
+}
+
+FUZZ_TARGET_INIT(base_encode_decode, initialize_base_encode_decode)
 {
    const std::string random_encoded_string(buffer.begin(), buffer.end());

--- a/src/test/fuzz/crypto.cpp
+++ b/src/test/fuzz/crypto.cpp
@ -19,6 +19,10 @@

 FUZZ_TARGET(crypto)
 {
+    // Hashing is expensive with sanitizers enabled, so limit the number of
+    // calls
+    int limit_max_ops{30};
+
    FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()};
    std::vector<uint8_t> data = ConsumeRandomLengthByteVector(fuzzed_data_provider);
    if (data.empty()) {
@ -36,7 +40,7 @@ FUZZ_TARGET(crypto)
    SHA3_256 sha3;
    CSipHasher sip_hasher{fuzzed_data_provider.ConsumeIntegral<uint64_t>(), fuzzed_data_provider.ConsumeIntegral<uint64_t>()};

-    while (fuzzed_data_provider.ConsumeBool()) {
+    while (--limit_max_ops >= 0 && fuzzed_data_provider.ConsumeBool()) {
        CallOneOf(
            fuzzed_data_provider,
            [&] {
--- a/src/test/fuzz/deserialize.cpp
+++ b/src/test/fuzz/deserialize.cpp
@ -133,8 +133,7 @@ FUZZ_TARGET_DESERIALIZE(partial_merkle_tree_deserialize, {
 FUZZ_TARGET_DESERIALIZE(pub_key_deserialize, {
        CPubKey pub_key;
        DeserializeFromFuzzingInput(buffer, pub_key);
-        // TODO: The following equivalence should hold for CPubKey? Fix.
-        // AssertEqualAfterSerializeDeserialize(pub_key);
+        AssertEqualAfterSerializeDeserialize(pub_key);
 })
 FUZZ_TARGET_DESERIALIZE(script_deserialize, {
        CScript script;
--- a/src/test/fuzz/prevector.cpp
+++ b/src/test/fuzz/prevector.cpp
@ -206,10 +206,14 @@ public:

 FUZZ_TARGET(prevector)
 {
+    // Pick an arbitrary upper bound to limit the runtime and avoid timeouts on
+    // inputs.
+    int limit_max_ops{3000};
+
    FuzzedDataProvider prov(buffer.data(), buffer.size());
    prevector_tester<8, int> test;

-    while (prov.remaining_bytes()) {
+    while (--limit_max_ops >= 0 && prov.remaining_bytes()) {
        switch (prov.ConsumeIntegralInRange<int>(0, 13 + 3 * (test.size() > 0))) {
        case 0:
            test.insert(prov.ConsumeIntegralInRange<size_t>(0, test.size()), prov.ConsumeIntegral<int>());
--- a/src/test/fuzz/rolling_bloom_filter.cpp
+++ b/src/test/fuzz/rolling_bloom_filter.cpp
@ -16,12 +16,16 @@

 FUZZ_TARGET(rolling_bloom_filter)
 {
+    // Pick an arbitrary upper bound to limit the runtime and avoid timeouts on
+    // inputs.
+    int limit_max_ops{3000};
+
    FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size());

    CRollingBloomFilter rolling_bloom_filter{
        fuzzed_data_provider.ConsumeIntegralInRange<unsigned int>(1, 1000),
        0.999 / fuzzed_data_provider.ConsumeIntegralInRange<unsigned int>(1, std::numeric_limits<unsigned int>::max())};
-    while (fuzzed_data_provider.remaining_bytes() > 0) {
+    while (--limit_max_ops >= 0 && fuzzed_data_provider.remaining_bytes() > 0) {
        CallOneOf(
            fuzzed_data_provider,
            [&] {
@ -32,13 +36,10 @@ FUZZ_TARGET(rolling_bloom_filter)
                assert(present);
            },
            [&] {
-                const std::optional<uint256> u256 = ConsumeDeserializable<uint256>(fuzzed_data_provider);
-                if (!u256) {
-                    return;
-                }
-                (void)rolling_bloom_filter.contains(*u256);
-                rolling_bloom_filter.insert(*u256);
-                const bool present = rolling_bloom_filter.contains(*u256);
+                const uint256 u256{ConsumeUInt256(fuzzed_data_provider)};
+                (void)rolling_bloom_filter.contains(u256);
+                rolling_bloom_filter.insert(u256);
+                const bool present = rolling_bloom_filter.contains(u256);
                assert(present);
            },
            [&] {
--- a/src/test/fuzz/util.h
+++ b/src/test/fuzz/util.h
@ -345,7 +345,7 @@ public:
            [&] {
                mode = "a+";
            });
-#ifdef _GNU_SOURCE
+#if defined _GNU_SOURCE && !defined __ANDROID__
        const cookie_io_functions_t io_hooks = {
            FuzzedFileProvider::read,
            FuzzedFileProvider::write,
--- a/src/wallet/db.cpp
+++ b/src/wallet/db.cpp
@ -12,7 +12,7 @@

 std::vector<fs::path> ListDatabases(const fs::path& wallet_dir)
 {
-    const size_t offset = wallet_dir.string().size() + 1;
+    const size_t offset = wallet_dir.string().size() + (wallet_dir == wallet_dir.root_name() ? 0 : 1);
    std::vector<fs::path> paths;
    boost::system::error_code ec;

--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@ -1922,7 +1922,11 @@ CWallet::ScanResult CWallet::ScanForWalletTransactions(const uint256& start_bloc
    double progress_current = progress_begin;
    int block_height = start_height;
    while (!fAbortRescan && !chain().shutdownRequested()) {
-        m_scanning_progress = (progress_current - progress_begin) / (progress_end - progress_begin);
+        if (progress_end - progress_begin > 0.0) {
+            m_scanning_progress = (progress_current - progress_begin) / (progress_end - progress_begin);
+        } else { // avoid divide-by-zero for single block scan range (i.e. start and stop hashes are equal)
+            m_scanning_progress = 0;
+        }
        if (block_height % 100 == 0 && progress_end - progress_begin > 0.0) {
            ShowProgress(strprintf("%s " + _("Rescanning...").translated, GetDisplayName()), std::max(1, std::min(99, (int)(m_scanning_progress * 100))));
        }
--- a/test/sanitizer_suppressions/ubsan
+++ b/test/sanitizer_suppressions/ubsan
@ -1,8 +1,3 @@
-# -fsanitize=undefined suppressions
-# =================================
-float-divide-by-zero:validation.cpp
-float-divide-by-zero:wallet/wallet.cpp
-
 # -fsanitize=integer suppressions
 # ===============================
 # Unsigned integer overflow occurs when the result of an unsigned integer