cleanup: Some more test cleanups, removing overly smart code.

Author: iphydf

testing/fuzzing/e2e_fuzz_test.cc

@@ -8,6 +8,7 @@
 #include <fstream>
 #include <vector>
 
+#include "../../toxcore/crypto_core.h"
 #include "../../toxcore/tox.h"
 #include "../../toxcore/tox_dispatch.h"
 #include "../../toxcore/tox_events.h"

testing/fuzzing/fuzz_support.cc

@@ -30,6 +30,16 @@ struct Network_Addr {
     size_t size;
 };
 
+System::System(std::unique_ptr<Tox_System> in_sys, std::unique_ptr<Memory> in_mem,
+    std::unique_ptr<Network> in_ns, std::unique_ptr<Random> in_rng)
+    : sys(std::move(in_sys))
+    , mem(std::move(in_mem))
+    , ns(std::move(in_ns))
+    , rng(std::move(in_rng))
+{
+}
+System::System(System &&) = default;
+
 System::~System() { }
 
 static int recv_common(Fuzz_Data &input, uint8_t *buf, size_t buf_len)

testing/fuzzing/fuzz_support.h

@@ -9,9 +9,9 @@
 #include <cstdlib>
 #include <deque>
 #include <memory>
-#include <vector>
 #include <unordered_map>
 #include <utility>
+#include <vector>
 
 #include "../../toxcore/tox.h"
 
@@ -20,8 +20,10 @@ struct Fuzz_Data {
     std::size_t size;
 
     Fuzz_Data(const uint8_t *input_data, std::size_t input_size)
-        : data(input_data), size(input_size)
-    {}
+        : data(input_data)
+        , size(input_size)
+    {
+    }
 
     Fuzz_Data &operator=(const Fuzz_Data &rhs) = delete;
     Fuzz_Data(const Fuzz_Data &rhs) = delete;
@@ -93,14 +95,20 @@ struct Fuzz_Data {
     }                                        \
     DECL = INPUT.consume(SIZE)
 
+#define CONSUME_OR_RETURN_VAL(DECL, INPUT, SIZE, VAL) \
+    if (INPUT.size < SIZE) {                          \
+        return VAL;                                   \
+    }                                                 \
+    DECL = INPUT.consume(SIZE)
+
 inline void fuzz_select_target(uint8_t selector, Fuzz_Data &input)
 {
     // The selector selected no function, so we do nothing and rely on the
     // fuzzer to come up with a better selector.
 }
 
 template <typename Arg, typename... Args>
-void fuzz_select_target(uint8_t selector, Fuzz_Data &input, Arg &&fn, Args &&... args)
+void fuzz_select_target(uint8_t selector, Fuzz_Data &input, Arg &&fn, Args &&...args)
 {
     if (selector == sizeof...(Args)) {
         return fn(input);
@@ -109,7 +117,7 @@ void fuzz_select_target(uint8_t selector, Fuzz_Data &input, Arg &&fn, Args &&...
 }
 
 template <typename... Args>
-void fuzz_select_target(const uint8_t *data, std::size_t size, Args &&... args)
+void fuzz_select_target(const uint8_t *data, std::size_t size, Args &&...args)
 {
     Fuzz_Data input{data, size};
 
@@ -127,6 +135,10 @@ struct System {
     std::unique_ptr<Network> ns;
     std::unique_ptr<Random> rng;
 
+    System(std::unique_ptr<Tox_System> sys, std::unique_ptr<Memory> mem,
+        std::unique_ptr<Network> ns, std::unique_ptr<Random> rng);
+    System(System &&);
+
     // Not inline because sizeof of the above 2 structs is not known everywhere.
     ~System();
 

testing/fuzzing/fuzz_tox.h

@@ -1,96 +1,17 @@
 /* SPDX-License-Identifier: GPL-3.0-or-later
- * Copyright © 2022 The TokTok team.
+ * Copyright © 2022-2024 The TokTok team.
  */
 
 #ifndef C_TOXCORE_TESTING_FUZZING_FUZZ_TOX_H
 #define C_TOXCORE_TESTING_FUZZING_FUZZ_TOX_H
 
-#include <cassert>
 #include <memory>
 
-#include "../../toxcore/DHT.h"
-#include "../../toxcore/logger.h"
 #include "../../toxcore/network.h"
-#include "fuzz_support.h"
 
 constexpr uint16_t SIZE_IP_PORT = SIZE_IP6 + sizeof(uint16_t);
 
 template <typename T>
 using Ptr = std::unique_ptr<T, void (*)(T *)>;
 
-/** @brief Construct any Tox resource using fuzzer input data.
- *
- * Constructs (or fails by returning) a valid object of type T and passes it to
- * a function specified on the rhs of `>>`. Takes care of cleaning up the
- * resource after the specified function returns.
- *
- * Some `with` instances require additional inputs such as the `Fuzz_Data`
- * reference or a logger.
- */
-template <typename T>
-struct with;
-
-/** @brief Construct a Logger without logging callback.
- */
-template <>
-struct with<Logger> {
-    template <typename F>
-    void operator>>(F &&f)
-    {
-        Ptr<Logger> logger(logger_new(), logger_kill);
-        assert(logger != nullptr);
-        f(std::move(logger));
-    }
-};
-
-/** @brief Construct an IP_Port by unpacking fuzzer input with `unpack_ip_port`.
- */
-template <>
-struct with<IP_Port> {
-    Fuzz_Data &input_;
-
-    template <typename F>
-    void operator>>(F &&f)
-    {
-        CONSUME_OR_RETURN(const uint8_t *ipp_packed, input_, SIZE_IP_PORT);
-        IP_Port ipp;
-        unpack_ip_port(&ipp, ipp_packed, SIZE_IP6, true);
-
-        f(ipp);
-    }
-};
-
-/** @brief Construct a Networking_Core object using the Network vtable passed.
- *
- * Use `with<Logger>{} >> with<Networking_Core>{input, ns, mem} >> ...` to construct
- * a logger and pass it to the Networking_Core constructor function.
- */
-template <>
-struct with<Networking_Core> {
-    Fuzz_Data &input_;
-    const Network *ns_;
-    const Memory *mem_;
-    Ptr<Logger> logger_{nullptr, logger_kill};
-
-    friend with operator>>(with<Logger> f, with self)
-    {
-        f >> [&self](Ptr<Logger> logger) { self.logger_ = std::move(logger); };
-        return self;
-    }
-
-    template <typename F>
-    void operator>>(F &&f)
-    {
-        with<IP_Port>{input_} >> [&f, this](const IP_Port &ipp) {
-            Ptr<Networking_Core> net(
-                new_networking_ex(logger_.get(), mem_, ns_, &ipp.ip, ipp.port, ipp.port + 100, nullptr),
-                kill_networking);
-            if (net == nullptr) {
-                return;
-            }
-            f(std::move(net));
-        };
-    }
-};
-
 #endif  // C_TOXCORE_TESTING_FUZZING_FUZZ_TOX_H

testing/fuzzing/protodump_reduce.cc

@@ -1,6 +1,7 @@
 #include <cassert>
 #include <cstdio>
 
+#include "../../toxcore/crypto_core.h"
 #include "../../toxcore/tox.h"
 #include "../../toxcore/tox_dispatch.h"
 #include "../../toxcore/tox_events.h"

toxcore/BUILD.bazel

@@ -296,6 +296,7 @@ cc_library(
     deps = [
         ":crypto_core",
         ":network",
+        ":test_util",
     ],
 )
 

toxcore/DHT_test.cc

@@ -333,13 +333,13 @@ TEST(AnnounceNodes, SetAndTest)
     ASSERT_NE(log, nullptr);
     Mono_Time *mono_time = mono_time_new(mem, nullptr, nullptr);
     ASSERT_NE(mono_time, nullptr);
-    Networking_Core *net = new_networking_no_udp(log, mem, ns);
+    Ptr<Networking_Core> net(new_networking_no_udp(log, mem, ns));
     ASSERT_NE(net, nullptr);
-    DHT *dht = new_dht(log, mem, rng, ns, mono_time, net, true, true);
+    Ptr<DHT> dht(new_dht(log, mem, rng, ns, mono_time, net.get(), true, true));
     ASSERT_NE(dht, nullptr);
 
     uint8_t pk_data[CRYPTO_PUBLIC_KEY_SIZE];
-    memcpy(pk_data, dht_get_self_public_key(dht), sizeof(pk_data));
+    memcpy(pk_data, dht_get_self_public_key(dht.get()), sizeof(pk_data));
     PublicKey self_pk(to_array(pk_data));
 
     PublicKey pk1 = random_pk(rng);
@@ -353,20 +353,20 @@ TEST(AnnounceNodes, SetAndTest)
     IP_Port ip_port = {0};
     ip_port.ip.family = net_family_ipv4();
 
-    set_announce_node(dht, pk1.data());
-    set_announce_node(dht, pk2.data());
+    set_announce_node(dht.get(), pk1.data());
+    set_announce_node(dht.get(), pk2.data());
 
-    EXPECT_TRUE(addto_lists(dht, &ip_port, pk1.data()));
-    EXPECT_TRUE(addto_lists(dht, &ip_port, pk2.data()));
+    EXPECT_TRUE(addto_lists(dht.get(), &ip_port, pk1.data()));
+    EXPECT_TRUE(addto_lists(dht.get(), &ip_port, pk2.data()));
 
     Node_format nodes[MAX_SENT_NODES];
-    EXPECT_EQ(0, get_close_nodes(dht, self_pk.data(), nodes, net_family_unspec(), true, true));
-    set_announce_node(dht, pk1.data());
-    set_announce_node(dht, pk2.data());
-    EXPECT_EQ(2, get_close_nodes(dht, self_pk.data(), nodes, net_family_unspec(), true, true));
+    EXPECT_EQ(
+        0, get_close_nodes(dht.get(), self_pk.data(), nodes, net_family_unspec(), true, true));
+    set_announce_node(dht.get(), pk1.data());
+    set_announce_node(dht.get(), pk2.data());
+    EXPECT_EQ(
+        2, get_close_nodes(dht.get(), self_pk.data(), nodes, net_family_unspec(), true, true));
 
-    kill_dht(dht);
-    kill_networking(net);
     mono_time_free(mem, mono_time);
     logger_kill(log);
 }

toxcore/DHT_test_util.hh

@@ -4,6 +4,10 @@
 #include <iosfwd>
 
 #include "DHT.h"
+#include "test_util.hh"
+
+template <>
+struct Deleter<DHT> : Function_Deleter<DHT, kill_dht> { };
 
 bool operator==(Node_format const &a, Node_format const &b);
 

toxcore/forwarding_fuzz_test.cc

@@ -1,47 +1,87 @@
 #include "forwarding.h"
 
 #include <cassert>
+#include <cstring>
 #include <memory>
+#include <optional>
 
 #include "../testing/fuzzing/fuzz_support.h"
 #include "../testing/fuzzing/fuzz_tox.h"
 
 namespace {
 
+std::optional<std::tuple<IP_Port, IP_Port, const uint8_t *, size_t>> prepare(Fuzz_Data &input)
+{
+    CONSUME_OR_RETURN_VAL(const uint8_t *ipp_packed, input, SIZE_IP_PORT, std::nullopt);
+    IP_Port ipp;
+    unpack_ip_port(&ipp, ipp_packed, SIZE_IP6, true);
+
+    CONSUME_OR_RETURN_VAL(const uint8_t *forwarder_packed, input, SIZE_IP_PORT, std::nullopt);
+    IP_Port forwarder;
+    unpack_ip_port(&forwarder, forwarder_packed, SIZE_IP6, true);
+
+    // 2 bytes: size of the request
+    CONSUME_OR_RETURN_VAL(const uint8_t *data_size_bytes, input, sizeof(uint16_t), std::nullopt);
+    uint16_t data_size;
+    std::memcpy(&data_size, data_size_bytes, sizeof(uint16_t));
+
+    // data bytes (max 64K)
+    CONSUME_OR_RETURN_VAL(const uint8_t *data, input, data_size, std::nullopt);
+
+    return {{ipp, forwarder, data, data_size}};
+}
+
 void TestSendForwardRequest(Fuzz_Data &input)
 {
-    const Network *ns = system_network();  // TODO(iphydf): fuzz_network
-    assert(ns != nullptr);
-    const Memory *mem = system_memory();  // TODO(iphydf): fuzz_memory
-    assert(mem != nullptr);
-
-    with<Logger>{} >> with<Networking_Core>{input, ns, mem} >> [&input](Ptr<Networking_Core> net) {
-        with<IP_Port>{input} >> [net = std::move(net), &input](const IP_Port &forwarder) {
-            CONSUME1_OR_RETURN(const uint16_t chain_length, input);
-            const uint16_t chain_keys_size = chain_length * CRYPTO_PUBLIC_KEY_SIZE;
-            CONSUME_OR_RETURN(const uint8_t *chain_keys, input, chain_keys_size);
-
-            send_forward_request(
-                net.get(), &forwarder, chain_keys, chain_length, input.data, input.size);
-        };
-    };
+    CONSUME1_OR_RETURN(const uint16_t chain_length, input);
+    const uint16_t chain_keys_size = chain_length * CRYPTO_PUBLIC_KEY_SIZE;
+    CONSUME_OR_RETURN(const uint8_t *chain_keys, input, chain_keys_size);
+
+    auto prep = prepare(input);
+    if (!prep.has_value()) {
+        return;
+    }
+    auto [ipp, forwarder, data, data_size] = prep.value();
+
+    // rest of the fuzz data is input for malloc and network
+    Fuzz_System sys(input);
+
+    Ptr<Logger> logger(logger_new(), logger_kill);
+
+    Ptr<Networking_Core> net(new_networking_ex(logger.get(), sys.mem.get(), sys.ns.get(), &ipp.ip,
+                                 ipp.port, ipp.port + 100, nullptr),
+        kill_networking);
+    if (net == nullptr) {
+        return;
+    }
+
+    send_forward_request(net.get(), &forwarder, chain_keys, chain_length, data, data_size);
 }
 
 void TestForwardReply(Fuzz_Data &input)
 {
-    const Network *ns = system_network();  // TODO(iphydf): fuzz_network
-    assert(ns != nullptr);
-    const Memory *mem = system_memory();  // TODO(iphydf): fuzz_memory
-    assert(mem != nullptr);
-
-    with<Logger>{} >> with<Networking_Core>{input, ns, mem} >> [&input](Ptr<Networking_Core> net) {
-        with<IP_Port>{input} >> [net = std::move(net), &input](const IP_Port &forwarder) {
-            CONSUME1_OR_RETURN(const uint16_t sendback_length, input);
-            CONSUME_OR_RETURN(const uint8_t *sendback, input, sendback_length);
-
-            forward_reply(net.get(), &forwarder, sendback, sendback_length, input.data, input.size);
-        };
-    };
+    CONSUME1_OR_RETURN(const uint16_t sendback_length, input);
+    CONSUME_OR_RETURN(const uint8_t *sendback, input, sendback_length);
+
+    auto prep = prepare(input);
+    if (!prep.has_value()) {
+        return;
+    }
+    auto [ipp, forwarder, data, data_size] = prep.value();
+
+    // rest of the fuzz data is input for malloc and network
+    Fuzz_System sys(input);
+
+    Ptr<Logger> logger(logger_new(), logger_kill);
+
+    Ptr<Networking_Core> net(new_networking_ex(logger.get(), sys.mem.get(), sys.ns.get(), &ipp.ip,
+                                 ipp.port, ipp.port + 100, nullptr),
+        kill_networking);
+    if (net == nullptr) {
+        return;
+    }
+
+    forward_reply(net.get(), &forwarder, sendback, sendback_length, data, data_size);
 }
 
 }  // namespace

toxcore/network_test_util.hh

@@ -1,10 +1,14 @@
 #ifndef C_TOXCORE_TOXCORE_NETWORK_TEST_UTIL_H
 #define C_TOXCORE_TOXCORE_NETWORK_TEST_UTIL_H
 
-#include <ostream>
+#include <iosfwd>
 
 #include "crypto_core.h"
 #include "network.h"
+#include "test_util.hh"
+
+template <>
+struct Deleter<Networking_Core> : Function_Deleter<Networking_Core, kill_networking> { };
 
 IP_Port random_ip_port(const Random *rng);