From b2cdf2a535f399333f80519e54501be224bdb1ed Mon Sep 17 00:00:00 2001 From: Jeka Pats Date: Wed, 3 Jul 2019 14:17:59 +0300 Subject: [PATCH 1/9] Add libFuzzer integration + report bug This commit places the basics for libFuzzer integration with one fuzzer which fuzzes the readMetadata function. The fuzzer is located at fuzz/read-metadata. To add more fuzzers please add them to ./fuzz directory as described in the README. Also a memory corruption bug is found using this fuzzer which might lead to additional bugs after fix is pushed. --- CMakeLists.txt | 9 +++++++++ README.md | 24 ++++++++++++++++++++++++ cmake/printSummary.cmake | 1 + fuzz/CMakeLists.txt | 14 ++++++++++++++ fuzz/read-metadata.cpp | 24 ++++++++++++++++++++++++ 5 files changed, 72 insertions(+) create mode 100644 fuzz/CMakeLists.txt create mode 100644 fuzz/read-metadata.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index aabb3dca..e4f0093d 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -35,6 +35,7 @@ option( EXIV2_ENABLE_BMFF "Build with BMFF support" option( EXIV2_BUILD_SAMPLES "Build sample applications" ON ) option( EXIV2_BUILD_EXIV2_COMMAND "Build exiv2 command-line executable" ON ) option( EXIV2_BUILD_UNIT_TESTS "Build unit tests" OFF ) +option( EXIV2_BUILD_FUZZ_TESTS "Build fuzz tests (libFuzzer)" OFF ) option( EXIV2_BUILD_DOC "Add 'doc' target to generate documentation" OFF ) # Only intended to be used by Exiv2 developers/contributors @@ -91,6 +92,14 @@ if( EXIV2_BUILD_UNIT_TESTS ) add_subdirectory ( unitTests ) endif() +if( EXIV2_BUILD_FUZZ_TESTS) + if ((NOT COMPILER_IS_CLANG) OR (NOT EXIV2_TEAM_USE_SANITIZERS)) + message(FATAL_ERROR "You need to build with Clang and sanitizers for the fuzzers to work. " + "Use Clang and -DEXIV2_TEAM_USE_SANITIZERS=ON") + endif() + add_subdirectory ( fuzz ) +endif() + if( EXIV2_BUILD_SAMPLES ) add_subdirectory( samples ) get_directory_property(SAMPLES DIRECTORY samples DEFINITION APPLICATIONS) diff --git a/README.md b/README.md index aa4dbe64..b7217b4d 100644 --- a/README.md +++ b/README.md @@ -64,6 +64,7 @@ The file ReadMe.txt in a build bundle describes how to install the library on th 3. [Unit tests](#4-3) 4. [Python tests](#4-4) 5. [Test Summary](#4-5) + 6. [Fuzzing](#4-6) 5. [Platform Notes](#5) 1. [Linux](#5-1) 2. [macOS](#5-2) @@ -1039,6 +1040,29 @@ $ cd /build $ make python_tests 2>&1 | grep FAIL ``` +### 4.6 Fuzzing + +The code for the fuzzers is in `exiv2dir/fuzz` + +To build the fuzzers, use the *cmake* option `-DEXIV2_BUILD_FUZZ_TESTS=ON` and `-DEXIV2_TEAM_USE_SANITIZERS=ON`. +Note that it only works with clang compiler as libFuzzer is integrate with clang > 6.0 + +To build the fuzzers: + +```bash +$ cd +$ rm -rf build-fuzz ; mkdir build-fuzz ; cd build-fuzz +$ cmake .. -DCMAKE_CXX_COMPILER=$(which clang++) -DEXIV2_BUILD_FUZZ_TESTS=ON -DEXIV2_TEAM_USE_SANITIZERS=ON +$ cmake --build . +``` + +To execute the fuzzers: + +```bash +cd /build-fuzz +bin/ # for example ./bin/read-metadata.cpp +``` + [TOC](#TOC)
diff --git a/cmake/printSummary.cmake b/cmake/printSummary.cmake index 15f9bbda..71e4a0cf 100644 --- a/cmake/printSummary.cmake +++ b/cmake/printSummary.cmake @@ -66,6 +66,7 @@ endif() OptionOutput( "Building exiv2 command: " EXIV2_BUILD_EXIV2_COMMAND ) OptionOutput( "Building samples: " EXIV2_BUILD_SAMPLES ) OptionOutput( "Building unit tests: " EXIV2_BUILD_UNIT_TESTS ) +OptionOutput( "Building fuzz tests: " EXIV2_BUILD_FUZZ_TESTS ) OptionOutput( "Building doc: " EXIV2_BUILD_DOC ) OptionOutput( "Building with coverage flags: " BUILD_WITH_COVERAGE ) OptionOutput( "Using ccache: " BUILD_WITH_CCACHE ) diff --git a/fuzz/CMakeLists.txt b/fuzz/CMakeLists.txt new file mode 100644 index 00000000..281ff570 --- /dev/null +++ b/fuzz/CMakeLists.txt @@ -0,0 +1,14 @@ + +macro(fuzzer name) + add_executable(${name} ${name}.cpp) + set_target_properties(${name} + PROPERTIES + COMPILE_FLAGS "-fsanitize=fuzzer" + LINK_FLAGS "-fsanitize=fuzzer") + target_link_libraries(${name} + PRIVATE + exiv2lib + ) +endmacro() + +fuzzer(read-metadata) \ No newline at end of file diff --git a/fuzz/read-metadata.cpp b/fuzz/read-metadata.cpp new file mode 100644 index 00000000..9f0b5979 --- /dev/null +++ b/fuzz/read-metadata.cpp @@ -0,0 +1,24 @@ +#include + +#include +#include +#include + + +extern "C" int LLVMFuzzerTestOneInput(const uint8_t * Data, size_t Size) +try { + Exiv2::Image::UniquePtr image = Exiv2::ImageFactory::open(Data, Size); + assert(image.get() != 0); + image->readMetadata(); + + Exiv2::ExifData &exifData = image->exifData(); + if (exifData.empty()) { + return -1; + } + + + return 0; +} +catch (Exiv2::Error& e) { + return -1; +} From bf786f4cc6f4d6b9dd3b63c6b8030a34b3409366 Mon Sep 17 00:00:00 2001 From: Kevin Backhouse Date: Sun, 11 Jul 2021 21:14:08 +0100 Subject: [PATCH 2/9] Updates to make fuzzing work. --- CMakeLists.txt | 2 +- cmake/compilerFlags.cmake | 4 +++- fuzz/CMakeLists.txt | 2 +- fuzz/read-metadata.cpp | 21 +++++++++++++-------- src/iptc.cpp | 4 ++-- src/jpgimage.cpp | 2 +- src/tiffvisitor_int.cpp | 2 ++ 7 files changed, 23 insertions(+), 14 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index e4f0093d..474c6d4c 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -92,7 +92,7 @@ if( EXIV2_BUILD_UNIT_TESTS ) add_subdirectory ( unitTests ) endif() -if( EXIV2_BUILD_FUZZ_TESTS) +if( EXIV2_BUILD_FUZZ_TESTS ) if ((NOT COMPILER_IS_CLANG) OR (NOT EXIV2_TEAM_USE_SANITIZERS)) message(FATAL_ERROR "You need to build with Clang and sanitizers for the fuzzers to work. " "Use Clang and -DEXIV2_TEAM_USE_SANITIZERS=ON") diff --git a/cmake/compilerFlags.cmake b/cmake/compilerFlags.cmake index 20f6ac53..8459fa0e 100644 --- a/cmake/compilerFlags.cmake +++ b/cmake/compilerFlags.cmake @@ -82,7 +82,9 @@ if ( MINGW OR UNIX OR MSYS ) # MINGW, Linux, APPLE, CYGWIN set(SANITIZER_FLAGS "-fno-omit-frame-pointer -fsanitize=address") endif() elseif( COMPILER_IS_CLANG ) - if ( CMAKE_CXX_COMPILER_VERSION VERSION_GREATER 4.9 ) + if ( EXIV2_BUILD_FUZZ_TESTS ) + set(SANITIZER_FLAGS "-fsanitize=fuzzer-no-link") + elseif ( CMAKE_CXX_COMPILER_VERSION VERSION_GREATER 4.9 ) set(SANITIZER_FLAGS "-fno-omit-frame-pointer -fsanitize=address,undefined -fno-sanitize-recover=all") elseif ( CMAKE_CXX_COMPILER_VERSION VERSION_GREATER 3.4 ) set(SANITIZER_FLAGS "-fno-omit-frame-pointer -fsanitize=address,undefined") diff --git a/fuzz/CMakeLists.txt b/fuzz/CMakeLists.txt index 281ff570..86209e9c 100644 --- a/fuzz/CMakeLists.txt +++ b/fuzz/CMakeLists.txt @@ -11,4 +11,4 @@ macro(fuzzer name) ) endmacro() -fuzzer(read-metadata) \ No newline at end of file +fuzzer(read-metadata) diff --git a/fuzz/read-metadata.cpp b/fuzz/read-metadata.cpp index 9f0b5979..088b3ef8 100644 --- a/fuzz/read-metadata.cpp +++ b/fuzz/read-metadata.cpp @@ -4,21 +4,26 @@ #include #include +extern "C" int LLVMFuzzerTestOneInput(const uint8_t * Data, size_t Size) { + // Invalid files generate a lot of warnings, so switch off logging. + Exiv2::LogMsg::setLevel(Exiv2::LogMsg::mute); -extern "C" int LLVMFuzzerTestOneInput(const uint8_t * Data, size_t Size) -try { + Exiv2::XmpParser::initialize(); + ::atexit(Exiv2::XmpParser::terminate); + + try { Exiv2::Image::UniquePtr image = Exiv2::ImageFactory::open(Data, Size); assert(image.get() != 0); + image->readMetadata(); Exiv2::ExifData &exifData = image->exifData(); if (exifData.empty()) { - return -1; + return -1; } + } catch(...) { + // Exiv2 throws an exception if the metadata is invalid. + } - - return 0; -} -catch (Exiv2::Error& e) { - return -1; + return 0; } diff --git a/src/iptc.cpp b/src/iptc.cpp index 9ae64d81..4252a988 100644 --- a/src/iptc.cpp +++ b/src/iptc.cpp @@ -474,13 +474,13 @@ namespace Exiv2 { #endif } } -#ifndef SUPPRESS_WARNINGS else { +#ifndef SUPPRESS_WARNINGS EXV_WARNING << "IPTC dataset " << IptcKey(dataSet, record) << " has invalid size " << sizeData << "; skipped.\n"; +#endif return 7; } -#endif pRead += sizeData; } diff --git a/src/jpgimage.cpp b/src/jpgimage.cpp index e722a797..6e6f0f96 100644 --- a/src/jpgimage.cpp +++ b/src/jpgimage.cpp @@ -181,7 +181,7 @@ namespace Exiv2 { #endif return -2; } -#ifndef EXIV2_DEBUG_MESSAGES +#ifdef EXIV2_DEBUG_MESSAGES if ( (dataSize & 1) && position + dataSize == static_cast(sizePsData)) { std::cerr << "Warning: " diff --git a/src/tiffvisitor_int.cpp b/src/tiffvisitor_int.cpp index 063cd250..ccf12a4d 100644 --- a/src/tiffvisitor_int.cpp +++ b/src/tiffvisitor_int.cpp @@ -1335,7 +1335,9 @@ namespace Exiv2 { tc->setStart(p); object->addChild(std::move(tc)); } else { +#ifndef SUPPRESS_WARNINGS EXV_WARNING << "Unable to handle tag " << tag << ".\n"; +#endif } p += 12; } From a3e4efe6b4fdfaab6ce10e05f533f9670c19cebc Mon Sep 17 00:00:00 2001 From: Kevin Backhouse Date: Mon, 12 Jul 2021 10:53:04 +0100 Subject: [PATCH 3/9] Rename fuzz target --- fuzz/CMakeLists.txt | 2 +- fuzz/{read-metadata.cpp => fuzz-read-print-write.cpp} | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename fuzz/{read-metadata.cpp => fuzz-read-print-write.cpp} (100%) diff --git a/fuzz/CMakeLists.txt b/fuzz/CMakeLists.txt index 86209e9c..378b67b1 100644 --- a/fuzz/CMakeLists.txt +++ b/fuzz/CMakeLists.txt @@ -11,4 +11,4 @@ macro(fuzzer name) ) endmacro() -fuzzer(read-metadata) +fuzzer(fuzz-read-print-write) diff --git a/fuzz/read-metadata.cpp b/fuzz/fuzz-read-print-write.cpp similarity index 100% rename from fuzz/read-metadata.cpp rename to fuzz/fuzz-read-print-write.cpp From a7602639ea4fce5365e33b915b3126641b6ee868 Mon Sep 17 00:00:00 2001 From: Kevin Backhouse Date: Mon, 12 Jul 2021 10:56:04 +0100 Subject: [PATCH 4/9] Add printing and writing to fuzzer. --- fuzz/fuzz-read-print-write.cpp | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/fuzz/fuzz-read-print-write.cpp b/fuzz/fuzz-read-print-write.cpp index 088b3ef8..b37b26ad 100644 --- a/fuzz/fuzz-read-print-write.cpp +++ b/fuzz/fuzz-read-print-write.cpp @@ -4,7 +4,7 @@ #include #include -extern "C" int LLVMFuzzerTestOneInput(const uint8_t * Data, size_t Size) { +extern "C" int LLVMFuzzerTestOneInput(const uint8_t * data, size_t size) { // Invalid files generate a lot of warnings, so switch off logging. Exiv2::LogMsg::setLevel(Exiv2::LogMsg::mute); @@ -12,15 +12,21 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t * Data, size_t Size) { ::atexit(Exiv2::XmpParser::terminate); try { - Exiv2::Image::UniquePtr image = Exiv2::ImageFactory::open(Data, Size); + Exiv2::DataBuf data_copy(data, size); + Exiv2::Image::UniquePtr image = + Exiv2::ImageFactory::open(data_copy.pData_, size); assert(image.get() != 0); image->readMetadata(); + image->exifData(); + + // Print to a std::ostringstream so that the fuzzer doesn't + // produce lots of garbage on stdout. + std::ostringstream buffer; + image->printStructure(buffer, Exiv2::kpsNone); + + image->writeMetadata(); - Exiv2::ExifData &exifData = image->exifData(); - if (exifData.empty()) { - return -1; - } } catch(...) { // Exiv2 throws an exception if the metadata is invalid. } From 95397cc17a1fe7116a13011cecd7cad687cb8f3e Mon Sep 17 00:00:00 2001 From: Kevin Backhouse Date: Mon, 12 Jul 2021 13:14:24 +0100 Subject: [PATCH 5/9] Action that runs fuzzer for short amount of time on PRs. --- .github/workflows/on_PR_linux_fuzz.yml | 28 ++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 .github/workflows/on_PR_linux_fuzz.yml diff --git a/.github/workflows/on_PR_linux_fuzz.yml b/.github/workflows/on_PR_linux_fuzz.yml new file mode 100644 index 00000000..66edbc15 --- /dev/null +++ b/.github/workflows/on_PR_linux_fuzz.yml @@ -0,0 +1,28 @@ +# Builds and runs the fuzz target for a short amount of time. This is +# mainly to protect the fuzz target from bitrot, but hopefully will +# also help to quickly catch some bugs before the PR is merged. + +name: Linux-Ubuntu Quick Fuzz on PRs + +on: [pull_request] + +jobs: + Linux: + name: 'Ubuntu 20.04 - clang/libFuzzer' + runs-on: ubuntu-latest + + steps: + - uses: actions/checkout@v2 + - name: install dependencies + run: ./ci/install_dependencies.sh + - name: build and compile + run: | + mkdir build && cd build + cmake -DEXIV2_ENABLE_PNG=ON -DEXIV2_ENABLE_WEBREADY=ON -DEXIV2_ENABLE_CURL=ON -DEXIV2_ENABLE_BMFF=ON -DEXIV2_TEAM_WARNINGS_AS_ERRORS=ON -DCMAKE_CXX_COMPILER=$(which clang++) -DEXIV2_BUILD_FUZZ_TESTS=ON -DEXIV2_TEAM_USE_SANITIZERS=ON .. + make -j $(nproc) + + - name: Fuzz + run: | + cd build + mkdir corpus + ./bin/fuzz-read-print-write corpus ../test/data/ -jobs=$(nproc) -max_total_time=60 -max_len=4096 From f4a0335d797c01120469ca3fc652096015bc83ee Mon Sep 17 00:00:00 2001 From: Kevin Backhouse Date: Mon, 12 Jul 2021 14:29:51 +0100 Subject: [PATCH 6/9] Add workflow_dispatch for manual trigger --- .github/workflows/on_PR_linux_fuzz.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/workflows/on_PR_linux_fuzz.yml b/.github/workflows/on_PR_linux_fuzz.yml index 66edbc15..3a47e131 100644 --- a/.github/workflows/on_PR_linux_fuzz.yml +++ b/.github/workflows/on_PR_linux_fuzz.yml @@ -4,7 +4,9 @@ name: Linux-Ubuntu Quick Fuzz on PRs -on: [pull_request] +on: + pull_request: + workflow_dispatch: jobs: Linux: From b4448fcd523bca2fd798e5d9b824dab21c58538e Mon Sep 17 00:00:00 2001 From: Kevin Backhouse Date: Mon, 12 Jul 2021 14:40:03 +0100 Subject: [PATCH 7/9] Fix indentation --- .github/workflows/on_PR_linux_fuzz.yml | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/.github/workflows/on_PR_linux_fuzz.yml b/.github/workflows/on_PR_linux_fuzz.yml index 3a47e131..ae1d0065 100644 --- a/.github/workflows/on_PR_linux_fuzz.yml +++ b/.github/workflows/on_PR_linux_fuzz.yml @@ -23,8 +23,8 @@ jobs: cmake -DEXIV2_ENABLE_PNG=ON -DEXIV2_ENABLE_WEBREADY=ON -DEXIV2_ENABLE_CURL=ON -DEXIV2_ENABLE_BMFF=ON -DEXIV2_TEAM_WARNINGS_AS_ERRORS=ON -DCMAKE_CXX_COMPILER=$(which clang++) -DEXIV2_BUILD_FUZZ_TESTS=ON -DEXIV2_TEAM_USE_SANITIZERS=ON .. make -j $(nproc) - - name: Fuzz - run: | - cd build - mkdir corpus - ./bin/fuzz-read-print-write corpus ../test/data/ -jobs=$(nproc) -max_total_time=60 -max_len=4096 + - name: Fuzz + run: | + cd build + mkdir corpus + ./bin/fuzz-read-print-write corpus ../test/data/ -jobs=$(nproc) -max_total_time=60 -max_len=4096 From 7eef360295a0d431ebee3dd6acfd89e859ec7376 Mon Sep 17 00:00:00 2001 From: Kevin Backhouse Date: Mon, 12 Jul 2021 15:59:17 +0100 Subject: [PATCH 8/9] Try with sudo. --- .github/workflows/on_PR_linux_fuzz.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/on_PR_linux_fuzz.yml b/.github/workflows/on_PR_linux_fuzz.yml index ae1d0065..8f02eac3 100644 --- a/.github/workflows/on_PR_linux_fuzz.yml +++ b/.github/workflows/on_PR_linux_fuzz.yml @@ -16,7 +16,7 @@ jobs: steps: - uses: actions/checkout@v2 - name: install dependencies - run: ./ci/install_dependencies.sh + run: sudo ./ci/install_dependencies.sh - name: build and compile run: | mkdir build && cd build From e157fd63cb5dd059d64bcde538f2c0f3f7343c22 Mon Sep 17 00:00:00 2001 From: Kevin Backhouse Date: Tue, 13 Jul 2021 13:13:45 +0100 Subject: [PATCH 9/9] Add -workers option to use all cores. --- .github/workflows/on_PR_linux_fuzz.yml | 2 +- README.md | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/.github/workflows/on_PR_linux_fuzz.yml b/.github/workflows/on_PR_linux_fuzz.yml index 8f02eac3..9fb1e5b2 100644 --- a/.github/workflows/on_PR_linux_fuzz.yml +++ b/.github/workflows/on_PR_linux_fuzz.yml @@ -27,4 +27,4 @@ jobs: run: | cd build mkdir corpus - ./bin/fuzz-read-print-write corpus ../test/data/ -jobs=$(nproc) -max_total_time=60 -max_len=4096 + ./bin/fuzz-read-print-write corpus ../test/data/ -jobs=$(nproc) -workers=$(nproc) -max_total_time=120 -max_len=4096 diff --git a/README.md b/README.md index b7217b4d..f0ae44a2 100644 --- a/README.md +++ b/README.md @@ -1056,11 +1056,12 @@ $ cmake .. -DCMAKE_CXX_COMPILER=$(which clang++) -DEXIV2_BUILD_FUZZ_TESTS=ON -DE $ cmake --build . ``` -To execute the fuzzers: +To execute a fuzzer: ```bash cd /build-fuzz -bin/ # for example ./bin/read-metadata.cpp +mkdir corpus +./bin/fuzz-read-print-write corpus ../test/data/ -jobs=$(nproc) -workers=$(nproc) -max_len=4096 ``` [TOC](#TOC)