Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add fuzzing support in libmaxminddb #357

Merged
merged 2 commits into from
Nov 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -44,3 +44,4 @@ Makefile
Makefile.in
Testing/
install_manifest.txt
build/
1 change: 1 addition & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ if (WIN32)
endif()
option(BUILD_SHARED_LIBS "Build shared libraries (.dll/.so) instead of static ones (.lib/.a)" OFF)
option(BUILD_TESTING "Build test programs" ON)
option(BUILD_FUZZING "Build with fuzzer" OFF)
option(MAXMINDDB_BUILD_BINARIES "Build binaries" ON)
option(MAXMINDDB_INSTALL "Generate the install target" ON)

Expand Down
41 changes: 41 additions & 0 deletions README.fuzzing.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
# Fuzzing libmaxminddb

These tests are only meant to be run on GNU/Linux.

## Build maxminddb fuzzer using libFuzzer.

### Export flags for fuzzing.

Note that in `CFLAGS` and `CXXFLAGS`, any type of sanitizers can be added.

- [AddressSanitizer](https://clang.llvm.org/docs/AddressSanitizer.html),
[ThreadSanitizer](https://clang.llvm.org/docs/ThreadSanitizer.html),
[MemorySanitizer](https://clang.llvm.org/docs/MemorySanitizer.html),
[UndefinedBehaviorSanitizer](https://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html),
[LeakSanitizer](https://clang.llvm.org/docs/LeakSanitizer.html).

```shell
$ export CC=clang
$ export CXX=clang++
$ export CFLAGS="-g -DFUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION -fsanitize=address,undefined -fsanitize=fuzzer-no-link"
$ export CXXFLAGS="-g -DFUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION -fsanitize=address,undefined -fsanitize=fuzzer-no-link"
$ export LIB_FUZZING_ENGINE="-fsanitize=fuzzer"
```

### Build maxminddb for fuzzing.

```shell
$ mkdir -p build && cd build
$ cmake -DBUILD_FUZZING=ON ../.
$ cmake --build . -j$(nproc)
```

### Running fuzzer.

```shell
$ mkdir -p fuzz_mmdb_seed fuzz_mmdb_seed_corpus
$ find ../t/maxmind-db/test-data/ -type f -size -4k -exec cp {} ./fuzz_mmdb_seed_corpus/ \;
$ ./t/fuzz_mmdb fuzz_mmdb_seed/ fuzz_mmdb_seed_corpus/
```

Here is more information about [LibFuzzer](https://llvm.org/docs/LibFuzzer.html).
6 changes: 6 additions & 0 deletions t/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -52,3 +52,9 @@ foreach(TEST_TARGET_NAME ${TEST_TARGET_NAMES})

add_test( NAME ${TEST_TARGET_NAME} COMMAND ${TEST_TARGET_NAME} WORKING_DIRECTORY ${PROJECT_SOURCE_DIR}/t)
endforeach()

if(BUILD_FUZZING)
add_executable(fuzz_mmdb fuzz_mmdb.c)
target_include_directories(fuzz_mmdb PRIVATE ../src)
target_link_libraries(fuzz_mmdb maxminddb $ENV{LIB_FUZZING_ENGINE})
endif()
36 changes: 36 additions & 0 deletions t/fuzz_mmdb.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
#include "maxminddb-compat-util.h"
#include "maxminddb.h"
#include <unistd.h>

#define kMinInputLength 2
#define kMaxInputLength 4048

extern int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size);

int
LLVMFuzzerTestOneInput(const uint8_t *data, size_t size)
{
int status;
FILE *fp;
MMDB_s mmdb;
char filename[256];

if (size < kMinInputLength || size > kMaxInputLength)
return 0;

sprintf(filename, "/tmp/libfuzzer.%d", getpid());

fp = fopen(filename, "wb");
if (!fp)
return 0;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this or the above return something other than 0?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this or the above return something other than 0?

No, if the program returns anything else other than return 0;, the coverage won't be counted.
I get it; we could use return -1; or return 1;, but I would say let's keep it return 0; for consistency.

Souce LibFuzzer

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would we want it to be counted though? It's an error, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's an error, right?

It is an error. I think there are bigger problems in your system if you see these errors.
We would be running in a controlled environment, so it won't be a problem.


fwrite(data, size, sizeof(uint8_t), fp);
fclose(fp);

status = MMDB_open(filename, MMDB_MODE_MMAP, &mmdb);
if (status == MMDB_SUCCESS)
MMDB_close(&mmdb);

unlink(filename);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would we want to error check any of these calls? This one, fwrite, fclose, sprintf could all have their return values checked I think.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one, fwrite, fclose, sprintf could all have their return values checked I think.

Good idea, but too much voodoo, I guess.

return 0;
}
Loading