From 50f1b30fa9544225a77053f8b5fefde48873b3f5 Mon Sep 17 00:00:00 2001 From: iphydf Date: Thu, 11 Jan 2024 15:24:02 +0000 Subject: [PATCH] test: Add fuzz tests to the coverage run. So we don't need to write so many edge case tests ourselves for things like parsers, which really don't need those manual tests, as long as we can check for some properties like "can output the parsed data and it'll be the same as the input". --- CMakeLists.txt | 43 ++++++++++++++--------------- cmake/ModulePackage.cmake | 13 --------- other/analysis/check_includes | 4 +-- other/docker/coverage/Dockerfile | 1 + testing/fuzzing/CMakeLists.txt | 29 +++++++++++-------- testing/fuzzing/fuzz_support.h | 22 ++++++++++----- toxcore/BUILD.bazel | 1 + toxcore/group_announce_fuzz_test.cc | 13 +++++---- toxcore/util_test.cc | 7 ++--- 9 files changed, 69 insertions(+), 64 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index ba173ec3..42211dce 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -160,21 +160,7 @@ if(BOOTSTRAP_DAEMON AND WIN32) set(BOOTSTRAP_DAEMON OFF) endif() -# Enabling this breaks all other tests and no network connections will be possible option(BUILD_FUZZ_TESTS "Build fuzzing harnesses" OFF) -if(BUILD_FUZZ_TESTS) - message(STATUS "Building in fuzz testing mode, no network connection will be possible") - # Disable everything we can - set(AUTOTEST OFF) - set(BUILD_MISC_TESTS OFF) - set(BUILD_FUN_UTILS OFF) - set(ENABLE_SHARED OFF) - set(MUST_BUILD_TOXAV OFF) - set(BUILD_TOXAV OFF) - set(BOOTSTRAP_DAEMON OFF) - set(DHT_BOOTSTRAP OFF) -endif() - if(MSVC) option(MSVC_STATIC_SODIUM "Whether to link libsodium statically for MSVC" OFF) @@ -448,20 +434,33 @@ endif() ################################################################################ # Create combined library from all the sources. -add_module(toxcore ${toxcore_SOURCES}) +if(ENABLE_SHARED) + add_library(toxcore_shared SHARED ${toxcore_SOURCES}) + set_target_properties(toxcore_shared PROPERTIES OUTPUT_NAME toxcore) + target_link_libraries(toxcore_shared PRIVATE ${toxcore_LINK_LIBRARIES}) + target_link_directories(toxcore_shared PUBLIC ${toxcore_LINK_DIRECTORIES}) + target_include_directories(toxcore_shared SYSTEM PRIVATE ${toxcore_INCLUDE_DIRECTORIES}) + target_compile_options(toxcore_shared PRIVATE ${toxcore_COMPILE_OPTIONS}) +endif() -# Link it to all dependencies. -if(TARGET toxcore_static) +if(ENABLE_STATIC) + add_library(toxcore_static STATIC ${toxcore_SOURCES}) + if(NOT MSVC) + set_target_properties(toxcore_static PROPERTIES OUTPUT_NAME toxcore) + endif() target_link_libraries(toxcore_static PRIVATE ${toxcore_LINK_LIBRARIES}) target_link_directories(toxcore_static PUBLIC ${toxcore_LINK_DIRECTORIES}) target_include_directories(toxcore_static SYSTEM PRIVATE ${toxcore_INCLUDE_DIRECTORIES}) target_compile_options(toxcore_static PRIVATE ${toxcore_COMPILE_OPTIONS}) endif() -if(TARGET toxcore_shared) - target_link_libraries(toxcore_shared PRIVATE ${toxcore_LINK_LIBRARIES}) - target_link_directories(toxcore_shared PUBLIC ${toxcore_LINK_DIRECTORIES}) - target_include_directories(toxcore_shared SYSTEM PRIVATE ${toxcore_INCLUDE_DIRECTORIES}) - target_compile_options(toxcore_shared PRIVATE ${toxcore_COMPILE_OPTIONS}) + +if(BUILD_FUZZ_TESTS) + add_library(toxcore_fuzz STATIC ${toxcore_SOURCES}) + target_link_libraries(toxcore_fuzz PRIVATE ${toxcore_LINK_LIBRARIES}) + target_link_directories(toxcore_fuzz PUBLIC ${toxcore_LINK_DIRECTORIES}) + target_include_directories(toxcore_fuzz SYSTEM PRIVATE ${toxcore_INCLUDE_DIRECTORIES}) + target_compile_options(toxcore_fuzz PRIVATE ${toxcore_COMPILE_OPTIONS}) + target_compile_definitions(toxcore_fuzz PUBLIC "FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION") endif() # Make version script (on systems that support it) to limit symbol visibility. diff --git a/cmake/ModulePackage.cmake b/cmake/ModulePackage.cmake index 332110d7..c20a80a1 100644 --- a/cmake/ModulePackage.cmake +++ b/cmake/ModulePackage.cmake @@ -19,19 +19,6 @@ if(FULLY_STATIC) set(ENABLE_STATIC ON) endif() -function(add_module lib) - if(ENABLE_SHARED) - add_library(${lib}_shared SHARED ${ARGN}) - set_target_properties(${lib}_shared PROPERTIES OUTPUT_NAME ${lib}) - endif() - if(ENABLE_STATIC) - add_library(${lib}_static STATIC ${ARGN}) - if(NOT MSVC) - set_target_properties(${lib}_static PROPERTIES OUTPUT_NAME ${lib}) - endif() - endif() -endfunction() - function(install_module lib) if(TARGET ${lib}_shared) set_target_properties(${lib}_shared PROPERTIES diff --git a/other/analysis/check_includes b/other/analysis/check_includes index 7e4e29fd..6d8245a6 100755 --- a/other/analysis/check_includes +++ b/other/analysis/check_includes @@ -31,8 +31,8 @@ out = (subprocess.run( errors = 0 for line in out.split("\n"): - # other/fun can do what it wants. - if "/fun/" in line: + # other/fun and mallocfail can do what they want. + if "/fun/" in line or "/mallocfail/" in line: continue filename, include = line.split(":", 1) # We only check headers. diff --git a/other/docker/coverage/Dockerfile b/other/docker/coverage/Dockerfile index ba292ba2..8a289869 100644 --- a/other/docker/coverage/Dockerfile +++ b/other/docker/coverage/Dockerfile @@ -60,6 +60,7 @@ RUN source .github/scripts/flags-coverage.sh \ -DSTRICT_ABI=ON \ -DAUTOTEST=ON \ -DPROXY_TEST=ON \ + -DBUILD_FUZZ_TESTS=ON \ -DUSE_IPV6=OFF \ -DTEST_TIMEOUT_SECONDS=40 \ && cmake --build _build --parallel 8 --target install diff --git a/testing/fuzzing/CMakeLists.txt b/testing/fuzzing/CMakeLists.txt index c2b88c72..d1fd7534 100644 --- a/testing/fuzzing/CMakeLists.txt +++ b/testing/fuzzing/CMakeLists.txt @@ -1,26 +1,33 @@ -# For coverage tests -target_compile_definitions(toxcore_static PUBLIC "FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION") - # Override network and random functions add_library(fuzz_support func_conversion.h fuzz_support.cc fuzz_support.h) set(LIBFUZZER_LINKER_FLAGS) -if (CMAKE_CXX_COMPILER_ID STREQUAL "Clang") +if(CMAKE_CXX_COMPILER_ID STREQUAL "Clang") set(LIBFUZZER_LINKER_FLAGS "-fsanitize=fuzzer") else() message(SEND_ERROR "Compiler must be Clang to build fuzz targets") endif() +function(fuzz_test target source_dir) + set(${target}_CORPUS_DIR ${CMAKE_CURRENT_SOURCE_DIR}/toktok-fuzzer/corpus/${target}_fuzz_test) + file(GLOB ${target}_fuzz_CORPUS "${${target}_CORPUS_DIR}/*") + add_executable(${target}_fuzz_test ${source_dir}/${target}_fuzz_test.cc) + target_link_libraries(${target}_fuzz_test PRIVATE toxcore_fuzz fuzz_support ${LIBFUZZER_LINKER_FLAGS}) + if(${target}_fuzz_CORPUS) + add_test(NAME ${target}_fuzz COMMAND ${CROSSCOMPILING_EMULATOR} ${target}_fuzz_test -max_total_time=10 ${${target}_fuzz_CORPUS}) + endif() +endfunction() + # Fuzzes the toxsave API add_executable(toxsave_fuzzer toxsave_harness.cc) -target_link_libraries(toxsave_fuzzer PRIVATE toxcore_static fuzz_support ${LIBFUZZER_LINKER_FLAGS}) +target_link_libraries(toxsave_fuzzer PRIVATE toxcore_fuzz fuzz_support ${LIBFUZZER_LINKER_FLAGS}) # Fuzzes the bootstrap process add_executable(bootstrap_fuzzer bootstrap_harness.cc) -target_link_libraries(bootstrap_fuzzer PRIVATE toxcore_static fuzz_support ${LIBFUZZER_LINKER_FLAGS}) +target_link_libraries(bootstrap_fuzzer PRIVATE toxcore_fuzz fuzz_support ${LIBFUZZER_LINKER_FLAGS}) -add_executable(DHT_fuzz_test ../../toxcore/DHT_fuzz_test.cc) -target_link_libraries(DHT_fuzz_test PRIVATE toxcore_static fuzz_support ${LIBFUZZER_LINKER_FLAGS}) - -add_executable(tox_events_fuzz_test ../../toxcore/tox_events_fuzz_test.cc) -target_link_libraries(tox_events_fuzz_test PRIVATE toxcore_static fuzz_support ${LIBFUZZER_LINKER_FLAGS}) +fuzz_test(DHT ../../toxcore) +fuzz_test(forwarding ../../toxcore) +fuzz_test(group_announce ../../toxcore) +fuzz_test(group_moderation ../../toxcore) +fuzz_test(tox_events ../../toxcore) diff --git a/testing/fuzzing/fuzz_support.h b/testing/fuzzing/fuzz_support.h index b1369db2..b08c9ead 100644 --- a/testing/fuzzing/fuzz_support.h +++ b/testing/fuzzing/fuzz_support.h @@ -7,6 +7,7 @@ #include #include +#include #include #include #include @@ -28,13 +29,20 @@ struct Fuzz_Data { Fuzz_Data &operator=(const Fuzz_Data &rhs) = delete; Fuzz_Data(const Fuzz_Data &rhs) = delete; - uint8_t consume1() - { - const uint8_t val = data[0]; - ++data; - --size; - return val; - } + struct Consumer { + Fuzz_Data &fd; + + template + operator T() + { + const uint8_t *bytes = fd.consume(sizeof(T)); + T val; + std::memcpy(&val, bytes, sizeof(T)); + return val; + } + }; + + Consumer consume1() { return Consumer{*this}; } const uint8_t *consume(std::size_t count) { diff --git a/toxcore/BUILD.bazel b/toxcore/BUILD.bazel index bf6bfba7..081a5b11 100644 --- a/toxcore/BUILD.bazel +++ b/toxcore/BUILD.bazel @@ -88,6 +88,7 @@ cc_test( srcs = ["util_test.cc"], deps = [ ":crypto_core", + ":crypto_core_test_util", ":util", "@com_google_googletest//:gtest", "@com_google_googletest//:gtest_main", diff --git a/toxcore/group_announce_fuzz_test.cc b/toxcore/group_announce_fuzz_test.cc index 6728e712..dab7c144 100644 --- a/toxcore/group_announce_fuzz_test.cc +++ b/toxcore/group_announce_fuzz_test.cc @@ -12,7 +12,8 @@ namespace { void TestUnpackAnnouncesList(Fuzz_Data &input) { CONSUME1_OR_RETURN(const uint8_t max_count, input); - std::vector announces(max_count); + // Always allocate at least something to avoid passing nullptr to functions below. + std::vector announces(max_count + 1); // TODO(iphydf): How do we know the packed size? CONSUME1_OR_RETURN(const uint16_t packed_size, input); @@ -20,10 +21,11 @@ void TestUnpackAnnouncesList(Fuzz_Data &input) Logger *logger = logger_new(); if (gca_unpack_announces_list(logger, input.data, input.size, announces.data(), max_count) != -1) { - std::vector packed(packed_size); + // Always allocate at least something to avoid passing nullptr to functions below. + std::vector packed(packed_size + 1); size_t processed; gca_pack_announces_list( - logger, packed.data(), packed.size(), announces.data(), announces.size(), &processed); + logger, packed.data(), packed_size, announces.data(), max_count, &processed); } logger_kill(logger); } @@ -37,8 +39,9 @@ void TestUnpackPublicAnnounce(Fuzz_Data &input) Logger *logger = logger_new(); if (gca_unpack_public_announce(logger, input.data, input.size, &public_announce) != -1) { - std::vector packed(packed_size); - gca_pack_public_announce(logger, packed.data(), packed.size(), &public_announce); + // Always allocate at least something to avoid passing nullptr to functions below. + std::vector packed(packed_size + 1); + gca_pack_public_announce(logger, packed.data(), packed_size, &public_announce); } logger_kill(logger); } diff --git a/toxcore/util_test.cc b/toxcore/util_test.cc index 47bf2587..aca278e3 100644 --- a/toxcore/util_test.cc +++ b/toxcore/util_test.cc @@ -3,13 +3,13 @@ #include #include "crypto_core.h" +#include "crypto_core_test_util.hh" namespace { TEST(Util, TwoRandomIdsAreNotEqual) { - const Random *rng = system_random(); - ASSERT_NE(rng, nullptr); + Test_Random rng; uint8_t pk1[CRYPTO_PUBLIC_KEY_SIZE]; uint8_t sk1[CRYPTO_SECRET_KEY_SIZE]; uint8_t pk2[CRYPTO_PUBLIC_KEY_SIZE]; @@ -23,8 +23,7 @@ TEST(Util, TwoRandomIdsAreNotEqual) TEST(Util, IdCopyMakesKeysEqual) { - const Random *rng = system_random(); - ASSERT_NE(rng, nullptr); + Test_Random rng; uint8_t pk1[CRYPTO_PUBLIC_KEY_SIZE]; uint8_t sk1[CRYPTO_SECRET_KEY_SIZE]; uint8_t pk2[CRYPTO_PUBLIC_KEY_SIZE] = {0};