From 32b68cffca603ffd9db1be6350060df0bdcc2324 Mon Sep 17 00:00:00 2001 From: iphydf Date: Wed, 10 Jan 2024 02:56:22 +0000 Subject: [PATCH] cleanup: Some more test cleanups, removing overly smart code. --- testing/fuzzing/e2e_fuzz_test.cc | 1 + testing/fuzzing/fuzz_support.cc | 10 +++ testing/fuzzing/fuzz_support.h | 22 +++++-- testing/fuzzing/fuzz_tox.h | 81 +----------------------- testing/fuzzing/protodump_reduce.cc | 1 + toxcore/BUILD.bazel | 1 + toxcore/DHT_test.cc | 26 ++++---- toxcore/DHT_test_util.hh | 4 ++ toxcore/forwarding_fuzz_test.cc | 96 ++++++++++++++++++++--------- toxcore/network_test_util.hh | 6 +- toxcore/test_util.hh | 12 ++++ 11 files changed, 133 insertions(+), 127 deletions(-) diff --git a/testing/fuzzing/e2e_fuzz_test.cc b/testing/fuzzing/e2e_fuzz_test.cc index f5258853e5..d19f0d6c60 100644 --- a/testing/fuzzing/e2e_fuzz_test.cc +++ b/testing/fuzzing/e2e_fuzz_test.cc @@ -8,6 +8,7 @@ #include #include +#include "../../toxcore/crypto_core.h" #include "../../toxcore/tox.h" #include "../../toxcore/tox_dispatch.h" #include "../../toxcore/tox_events.h" diff --git a/testing/fuzzing/fuzz_support.cc b/testing/fuzzing/fuzz_support.cc index 04a0bf7427..aaf6b18a60 100644 --- a/testing/fuzzing/fuzz_support.cc +++ b/testing/fuzzing/fuzz_support.cc @@ -30,6 +30,16 @@ struct Network_Addr { size_t size; }; +System::System(std::unique_ptr in_sys, std::unique_ptr in_mem, + std::unique_ptr in_ns, std::unique_ptr 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) diff --git a/testing/fuzzing/fuzz_support.h b/testing/fuzzing/fuzz_support.h index a20d85b4f2..b1369db273 100644 --- a/testing/fuzzing/fuzz_support.h +++ b/testing/fuzzing/fuzz_support.h @@ -9,9 +9,9 @@ #include #include #include -#include #include #include +#include #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,6 +95,12 @@ 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 @@ -100,7 +108,7 @@ inline void fuzz_select_target(uint8_t selector, Fuzz_Data &input) } template -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 -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 ns; std::unique_ptr rng; + System(std::unique_ptr sys, std::unique_ptr mem, + std::unique_ptr ns, std::unique_ptr rng); + System(System &&); + // Not inline because sizeof of the above 2 structs is not known everywhere. ~System(); diff --git a/testing/fuzzing/fuzz_tox.h b/testing/fuzzing/fuzz_tox.h index b5864e78d3..c19bb1bd3a 100644 --- a/testing/fuzzing/fuzz_tox.h +++ b/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 #include -#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 using Ptr = std::unique_ptr; -/** @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 -struct with; - -/** @brief Construct a Logger without logging callback. - */ -template <> -struct with { - template - void operator>>(F &&f) - { - Ptr 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 { - Fuzz_Data &input_; - - template - 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{} >> with{input, ns, mem} >> ...` to construct - * a logger and pass it to the Networking_Core constructor function. - */ -template <> -struct with { - Fuzz_Data &input_; - const Network *ns_; - const Memory *mem_; - Ptr logger_{nullptr, logger_kill}; - - friend with operator>>(with f, with self) - { - f >> [&self](Ptr logger) { self.logger_ = std::move(logger); }; - return self; - } - - template - void operator>>(F &&f) - { - with{input_} >> [&f, this](const IP_Port &ipp) { - Ptr 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 diff --git a/testing/fuzzing/protodump_reduce.cc b/testing/fuzzing/protodump_reduce.cc index 847ac0b222..8cc475bb7e 100644 --- a/testing/fuzzing/protodump_reduce.cc +++ b/testing/fuzzing/protodump_reduce.cc @@ -1,6 +1,7 @@ #include #include +#include "../../toxcore/crypto_core.h" #include "../../toxcore/tox.h" #include "../../toxcore/tox_dispatch.h" #include "../../toxcore/tox_events.h" diff --git a/toxcore/BUILD.bazel b/toxcore/BUILD.bazel index 8f8d34afd6..bf6bfba78f 100644 --- a/toxcore/BUILD.bazel +++ b/toxcore/BUILD.bazel @@ -296,6 +296,7 @@ cc_library( deps = [ ":crypto_core", ":network", + ":test_util", ], ) diff --git a/toxcore/DHT_test.cc b/toxcore/DHT_test.cc index 5f957fe322..958125824f 100644 --- a/toxcore/DHT_test.cc +++ b/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 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(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); } diff --git a/toxcore/DHT_test_util.hh b/toxcore/DHT_test_util.hh index 6f00d49a57..5fa75b7f70 100644 --- a/toxcore/DHT_test_util.hh +++ b/toxcore/DHT_test_util.hh @@ -4,6 +4,10 @@ #include #include "DHT.h" +#include "test_util.hh" + +template <> +struct Deleter : Function_Deleter { }; bool operator==(Node_format const &a, Node_format const &b); diff --git a/toxcore/forwarding_fuzz_test.cc b/toxcore/forwarding_fuzz_test.cc index ce263f1e6f..03fed47419 100644 --- a/toxcore/forwarding_fuzz_test.cc +++ b/toxcore/forwarding_fuzz_test.cc @@ -1,47 +1,87 @@ #include "forwarding.h" #include +#include #include +#include #include "../testing/fuzzing/fuzz_support.h" #include "../testing/fuzzing/fuzz_tox.h" namespace { +std::optional> 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{} >> with{input, ns, mem} >> [&input](Ptr net) { - with{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_new(), logger_kill); + + Ptr 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{} >> with{input, ns, mem} >> [&input](Ptr net) { - with{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_new(), logger_kill); + + Ptr 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 diff --git a/toxcore/network_test_util.hh b/toxcore/network_test_util.hh index 4e55a52ad2..6e51a29945 100644 --- a/toxcore/network_test_util.hh +++ b/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 +#include #include "crypto_core.h" #include "network.h" +#include "test_util.hh" + +template <> +struct Deleter : Function_Deleter { }; IP_Port random_ip_port(const Random *rng); diff --git a/toxcore/test_util.hh b/toxcore/test_util.hh index c5eb6d9c6a..4ab2637e39 100644 --- a/toxcore/test_util.hh +++ b/toxcore/test_util.hh @@ -3,8 +3,20 @@ #include #include +#include #include +template +struct Function_Deleter { + void operator()(T *ptr) const { Delete(ptr); } +}; + +template +struct Deleter; + +template +using Ptr = std::unique_ptr>; + template std::array to_array(T const (&arr)[N]) {