From 82ca52af3ab4e6b94f54913254ceef805cd12e3c Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Thu, 29 May 2025 00:59:14 +0000 Subject: [PATCH 1/2] Add fuzz testing for BMP header loading This commit introduces fuzz testing for the `BmpTool::load()` function, specifically targeting 54-byte BMP headers to ensure graceful failure without heap interaction when processing malformed headers. Key changes: - Integrated libFuzzer into the CMake build system with an `ENABLE_FUZZING` option. This uses Clang and AddressSanitizer when enabled. - Created a fuzzing harness (`tests/fuzz/fuzz_bitmap.cpp`) that feeds 54-byte data chunks from the fuzzer to `BmpTool::load()`. - Added a GitHub Actions workflow (`.github/workflows/fuzzing.yml`) to automate the fuzzing process on pushes and pull requests. The workflow builds with Clang, ASan, and runs the fuzzer for a short duration. - Added a unit test (`tests/test_bitmap.cpp`) to specifically check for correct error handling (`BitmapError::NotABmp`) when an invalid BMP magic number is encountered. Manual analysis of `BmpTool::load()` suggests it is designed to prevent heap allocations if header validation fails for 54-byte inputs. The primary risks identified (integer overflows in validation, mishandling of `ih.biSize`) are targets for the automated fuzzing setup. A Clang compiler crash was encountered in the development environment when compiling `src/bitmap/bitmap.cpp` with fuzzing flags. This prevented direct execution and iteration of the fuzzer during this development, but the fuzzing infrastructure is provided for environments where this compilation succeeds (e.g., the GitHub Actions runner). --- .github/workflows/fuzzing.yml | 40 +++++++++++++++++++++++ CMakeLists.txt | 24 +++++++++++++- tests/CMakeLists.txt | 7 ++++ tests/fuzz/fuzz_bitmap.cpp | 23 +++++++++++++ tests/test_bitmap.cpp | 61 ++++++++++++++++++++++++++++++++++- 5 files changed, 153 insertions(+), 2 deletions(-) create mode 100644 .github/workflows/fuzzing.yml create mode 100644 tests/fuzz/fuzz_bitmap.cpp diff --git a/.github/workflows/fuzzing.yml b/.github/workflows/fuzzing.yml new file mode 100644 index 0000000..dfadf9d --- /dev/null +++ b/.github/workflows/fuzzing.yml @@ -0,0 +1,40 @@ +name: Fuzzing + +on: + push: + branches: [ main, master ] + pull_request: + branches: [ main, master ] + +jobs: + fuzz: + runs-on: ubuntu-latest + + steps: + - name: Checkout code + uses: actions/checkout@v4 + + - name: Install Clang + run: | + sudo apt-get update + sudo apt-get install -y clang llvm + + - name: Configure CMake for Fuzzing + run: | + cmake -S . -B build_fuzz -DENABLE_FUZZING=ON -DCMAKE_BUILD_TYPE=Debug + env: + CC: clang + CXX: clang++ + + - name: Build fuzz target + run: cmake --build build_fuzz --target fuzz_bitmap --config Debug + + - name: Create corpus directory + run: mkdir -p build_fuzz/corpus_fuzzing # Separate from build-time corpus + + - name: Run fuzzer + run: | + ./build_fuzz/tests/fuzz_bitmap -max_total_time=60 -print_final_stats=1 -error_exitcode=1 build_fuzz/corpus_fuzzing/ + # Optionally, if you want to use existing corpus from the repo (e.g., tests/fuzz/corpus) + # ./build_fuzz/tests/fuzz_bitmap -max_total_time=60 -print_final_stats=1 -error_exitcode=1 build_fuzz/corpus_fuzzing/ tests/fuzz/corpus/ + # If the fuzzer finds a crash, error_exitcode=1 will make the step fail. diff --git a/CMakeLists.txt b/CMakeLists.txt index 21fc6f6..83303a7 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1,9 +1,31 @@ cmake_minimum_required(VERSION 3.10) project(bitmap VERSION 1.0.0) +option(ENABLE_FUZZING "Enable fuzzing builds" OFF) + set(CMAKE_CXX_STANDARD 20) set(CMAKE_CXX_STANDARD_REQUIRED True) -set(CMAKE_CXX_EXTENSIONS OFF) +set(CMAKE_CXX_EXTENSIONS OFF) + +if(ENABLE_FUZZING) + if(NOT CMAKE_CXX_COMPILER_ID MATCHES "Clang") + find_program(CLANG_COMPILER NAMES clang++ clang) + if(CLANG_COMPILER) + set(CMAKE_CXX_COMPILER ${CLANG_COMPILER}) + else() + message(WARNING "Clang compiler not found. Disabling fuzzing.") + set(ENABLE_FUZZING OFF) + endif() + endif() + + if(ENABLE_FUZZING AND CMAKE_CXX_COMPILER_ID MATCHES "Clang") + message(STATUS "Fuzzing enabled. Using Clang compiler.") + set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -g -fsanitize=address,fuzzer-no-link") + # For linking fuzz targets, we'll use -fsanitize=fuzzer later + else() + message(STATUS "Fuzzing disabled or Clang not used.") + endif() +endif() include(CTest) enable_testing() diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index f0d21fa..8245cf5 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -27,3 +27,10 @@ target_link_libraries(bitmap_tests PRIVATE bitmap gtest_main) # Use CTest's test discovery for GoogleTest include(GoogleTest) gtest_discover_tests(bitmap_tests) + +if(ENABLE_FUZZING AND CMAKE_CXX_COMPILER_ID MATCHES "Clang") + add_executable(fuzz_bitmap fuzz/fuzz_bitmap.cpp) + target_link_libraries(fuzz_bitmap PRIVATE bitmap) + set_target_properties(fuzz_bitmap PROPERTIES LINK_FLAGS "-fsanitize=address,fuzzer") + message(STATUS "Fuzz target fuzz_bitmap added.") +endif() diff --git a/tests/fuzz/fuzz_bitmap.cpp b/tests/fuzz/fuzz_bitmap.cpp new file mode 100644 index 0000000..ed4f8b9 --- /dev/null +++ b/tests/fuzz/fuzz_bitmap.cpp @@ -0,0 +1,23 @@ +#include "../../include/bitmap.hpp" // For BmpTool::load and BmpTool::BitmapError +#include +#include // Required by span +#include // For std::span + +extern "C" int LLVMFuzzerTestOneInput(const uint8_t *Data, size_t Size) { + // We are specifically testing 54-byte headers. + // The fuzzer might provide smaller or larger inputs. + // We truncate or ignore inputs not of this size to focus the fuzzing. + if (Size < 54) { + return 0; // Not enough data for a 54-byte header. + } + + // Create a span for the 54-byte header. + std::span bmp_data(Data, 54); + + // Call the function to be fuzzed. + // We don't need to check the result for fuzzing purposes, + // as ASan/libFuzzer will report crashes or memory errors. + [[maybe_unused]] auto result = BmpTool::load(bmp_data); + + return 0; // Essential for libFuzzer to continue. +} diff --git a/tests/test_bitmap.cpp b/tests/test_bitmap.cpp index eae6b86..1e0c276 100644 --- a/tests/test_bitmap.cpp +++ b/tests/test_bitmap.cpp @@ -4,7 +4,13 @@ #include // Include the header for the code to be tested -#include "../src/bitmap/bitmap.h" +#include "../src/bitmap/bitmap.h" // Original include for Pixel, etc. +#include "../../include/bitmap.hpp" // For BmpTool::load and BmpTool::BitmapError + +#include // For std::array +#include // For std::span +#include // For fixed-width integer types +#include // For std::memcpy // Overload for Pixel struct comparison bool operator==(const Pixel& p1, const Pixel& p2) { @@ -492,3 +498,56 @@ TEST(BitmapTest, ChangePixelLuminanceCyan) { Pixel p_not_cyan = {20, 30, 150, 255}; // R is dominant EXPECT_EQ(p_not_cyan, ChangePixelLuminanceCyan(p_not_cyan, lum_factor)); } + +// New Test Suite for BmpTool specific tests +TEST(BmpToolLoadTest, LoadWithInvalidMagicType) { + std::array header_data{}; // Zero-initialize + + // Set invalid magic type + header_data[0] = 'X'; + header_data[1] = 'Y'; + + // Fill in plausible values for other critical fields to avoid premature errors + // that might mask the magic type check. + + // BITMAPFILEHEADER (14 bytes total) + // bfSize (offset 2, size 4): Total size of file. Let's say 54 (header only) + (16*16*3) for a dummy 16x16 24bpp image. + uint32_t dummy_file_size = 54 + (16 * 16 * 3); + std::memcpy(&header_data[2], &dummy_file_size, sizeof(dummy_file_size)); + // bfOffBits (offset 10, size 4): Offset to pixel data. Standard is 54 for no palette. + uint32_t off_bits = 54; + std::memcpy(&header_data[10], &off_bits, sizeof(off_bits)); + + // BITMAPINFOHEADER (starts at byte 14, 40 bytes total) + // biSize (offset 14, size 4): Size of BITMAPINFOHEADER, should be 40. + uint32_t info_header_size = 40; + std::memcpy(&header_data[14], &info_header_size, sizeof(info_header_size)); + // biWidth (offset 18, size 4): e.g., 16 + int32_t width = 16; + std::memcpy(&header_data[18], &width, sizeof(width)); + // biHeight (offset 22, size 4): e.g., 16 + int32_t height = 16; + std::memcpy(&header_data[22], &height, sizeof(height)); + // biPlanes (offset 26, size 2): must be 1 + uint16_t planes = 1; + std::memcpy(&header_data[26], &planes, sizeof(planes)); + // biBitCount (offset 28, size 2): e.g., 24 + uint16_t bit_count = 24; + std::memcpy(&header_data[28], &bit_count, sizeof(bit_count)); + // biCompression (offset 30, size 4): must be 0 (BI_RGB) for uncompressed + uint32_t compression = 0; // BI_RGB + std::memcpy(&header_data[30], &compression, sizeof(compression)); + // biSizeImage (offset 34, size 4): image size in bytes. (16*16*3) + uint32_t image_size = 16 * 16 * 3; + std::memcpy(&header_data[34], &image_size, sizeof(image_size)); + // biXPelsPerMeter (offset 38, size 4): Optional, can be 0 + // biYPelsPerMeter (offset 42, size 4): Optional, can be 0 + // biClrUsed (offset 46, size 4): Optional, can be 0 for 24bpp + // biClrImportant (offset 50, size 4): Optional, can be 0 + + std::span data_span(header_data.data(), header_data.size()); + auto result = BmpTool::load(data_span); + + ASSERT_TRUE(result.has_error()); + EXPECT_EQ(result.error(), BmpTool::BitmapError::NotABmp); +} From 4b55b35b3d9ae441bbbaf4d6712c3ba937ef65c2 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Thu, 29 May 2025 01:03:47 +0000 Subject: [PATCH 2/2] Fix: Correct method name in bitmap load test The method `result.has_error()` was incorrectly used in the `LoadWithInvalidMagicType` test case in `tests/test_bitmap.cpp`. This has been changed to the correct method `result.isError()`, as indicated by build failures in the CI. --- tests/test_bitmap.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_bitmap.cpp b/tests/test_bitmap.cpp index 1e0c276..f237d79 100644 --- a/tests/test_bitmap.cpp +++ b/tests/test_bitmap.cpp @@ -548,6 +548,6 @@ TEST(BmpToolLoadTest, LoadWithInvalidMagicType) { std::span data_span(header_data.data(), header_data.size()); auto result = BmpTool::load(data_span); - ASSERT_TRUE(result.has_error()); + ASSERT_TRUE(result.isError()); EXPECT_EQ(result.error(), BmpTool::BitmapError::NotABmp); }