Skip to content

Commit 6df3fe8

Browse files
committed
feat(mdns): Implement effective fuzzing guidelines
1 parent 584bf7d commit 6df3fe8

7 files changed

Lines changed: 288 additions & 63 deletions

File tree

.github/workflows/mdns__host-tests.yml

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -112,10 +112,12 @@ jobs:
112112
113113
fuzz_test:
114114
if: contains(github.event.pull_request.labels.*.name, 'mdns-fuzz') || github.event_name == 'push'
115-
name: Fuzzer tests for mdns lib
115+
name: Fuzzer (${{ matrix.fuzz_target }})
116116
strategy:
117+
fail-fast: false
117118
matrix:
118119
idf_ver: ["latest"]
120+
fuzz_target: [receive, browse]
119121

120122
runs-on: ubuntu-22.04
121123
container: aflplusplus/aflplusplus:v4.34c
@@ -135,20 +137,27 @@ jobs:
135137
apt-get update -y
136138
apt-get install -y libbsd-dev
137139
138-
- name: Run AFL++
140+
- name: Run AFL++ (${{ matrix.fuzz_target }})
139141
shell: bash
142+
env:
143+
ASAN_OPTIONS: abort_on_error=1:halt_on_error=1:symbolize=0:detect_stack_use_after_return=1:max_malloc_fill_size=1073741824
144+
UBSAN_OPTIONS: halt_on_error=1:abort_on_error=1
140145
run: |
141146
export IDF_PATH=$GITHUB_WORKSPACE/idf
142147
cd components/mdns/tests/host_unit_test/
143148
pip install dnslib
144149
cd input && python generate_cases.py && cd ..
145-
cmake -B build2 -S . -G "Ninja" -DCMAKE_C_COMPILER=afl-cc
146-
cmake --build build2
147-
timeout 10m afl-fuzz -i input -o out -- build2/mdns_host_unit_test || \
150+
BUILD_DIR=build_fuzz_${{ matrix.fuzz_target }}
151+
OUT_DIR=out_${{ matrix.fuzz_target }}
152+
cmake -B "${BUILD_DIR}" -S . -G Ninja \
153+
-DCMAKE_C_COMPILER=afl-clang-fast \
154+
-DFUZZ_TARGET=${{ matrix.fuzz_target }}
155+
cmake --build "${BUILD_DIR}"
156+
timeout 10m afl-fuzz -i input -o "${OUT_DIR}" -- "${BUILD_DIR}/mdns_host_unit_test" || \
148157
if [ $? -eq 124 ]; then # timeout exit code
149-
if [ -n "$(find out/default/crashes -type f 2>/dev/null)" ]; then
158+
if [ -n "$(find "${OUT_DIR}/default/crashes" -type f 2>/dev/null)" ]; then
150159
echo "Crashes found!";
151-
tar -czf out/default/crashes.tar.gz -C out/default crashes;
160+
tar -czf "${OUT_DIR}/default/crashes.tar.gz" -C "${OUT_DIR}/default" crashes;
152161
exit 1;
153162
fi
154163
else
@@ -159,6 +168,6 @@ jobs:
159168
if: failure()
160169
uses: actions/upload-artifact@v4
161170
with:
162-
name: fuzz-crashes
163-
path: components/mdns/tests/host_unit_test/out/default/crashes.tar.gz
171+
name: fuzz-crashes-${{ matrix.fuzz_target }}
172+
path: components/mdns/tests/host_unit_test/out_${{ matrix.fuzz_target }}/default/crashes.tar.gz
164173
if-no-files-found: ignore

ci/ignore_build_warnings.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,3 +8,4 @@ warning: unknown kconfig symbol 'EXAMPLE_ETH_PHY_IP101'
88
WARNING: The following Kconfig variables were used in "if" clauses, but not
99
warning: unknown kconfig symbol 'LIBC_NEWLIB'
1010
warning: 'MBEDTLS_PSA_BUILTIN_[A-Z0-9_]+' redefined
11+
The smallest .+ partition is nearly full \(\d+% free space left\)!

components/mdns/tests/host_unit_test/CMakeLists.txt

Lines changed: 25 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,16 @@ endif()
1010

1111
project(mdns_host_unit_test C)
1212

13-
# Set ENABLE_UNIT_TESTS with a default of OFF
14-
if(NOT DEFINED UNIT_TESTS)
15-
set(UNIT_TESTS "OFF" CACHE STRING "Unit tests: OFF, test_receiver, test_sender")
16-
else()
13+
# Unit tests are enabled only for a real suite name (fuzz builds leave this OFF).
14+
set(UNIT_TESTS "OFF" CACHE STRING "Unit tests: OFF, test_receiver, test_sender, test_browse")
15+
set_property(CACHE UNIT_TESTS PROPERTY STRINGS OFF test_receiver test_sender test_browse)
16+
17+
if(UNIT_TESTS AND NOT UNIT_TESTS STREQUAL "OFF")
1718
set(ENABLE_UNIT_TESTS 1)
1819
message(STATUS "Unit testing enabled with UNIT_TESTS=${UNIT_TESTS}")
20+
else()
21+
unset(ENABLE_UNIT_TESTS)
22+
message(STATUS "Unit testing disabled (fuzz/harness build)")
1923
endif()
2024

2125
# Set variables for directories
@@ -69,6 +73,17 @@ set(SOURCES
6973
if(ENABLE_UNIT_TESTS)
7074
include(unity/unit_test.cmake)
7175
else()
76+
# Fuzz harness target: receive (parse/query) or browse (cache/TXT path)
77+
set(FUZZ_TARGET "receive" CACHE STRING "Fuzz harness: receive, browse")
78+
set_property(CACHE FUZZ_TARGET PROPERTY STRINGS receive browse)
79+
if(FUZZ_TARGET STREQUAL "browse")
80+
add_definitions(-DFUZZ_TARGET_BROWSE=1)
81+
elseif(FUZZ_TARGET STREQUAL "receive")
82+
add_definitions(-DFUZZ_TARGET_RECEIVE=1)
83+
else()
84+
message(FATAL_ERROR "FUZZ_TARGET must be 'receive' or 'browse' (got '${FUZZ_TARGET}')")
85+
endif()
86+
message(STATUS "Fuzz harness target: ${FUZZ_TARGET}")
7287
list(APPEND SOURCES main.c)
7388
endif()
7489

@@ -84,8 +99,12 @@ endif()
8499

85100
# Sanitizers for AFL fuzzing (unit tests enable these in unity/enable_testing.cmake)
86101
if(NOT ENABLE_UNIT_TESTS)
87-
target_compile_options(${PROJECT_NAME} PRIVATE -fsanitize=address -fsanitize=undefined)
88-
target_link_options(${PROJECT_NAME} PRIVATE -fsanitize=address -fsanitize=undefined)
102+
target_compile_options(${PROJECT_NAME} PRIVATE
103+
-g3
104+
-fsanitize=address,undefined
105+
-fno-omit-frame-pointer
106+
)
107+
target_link_options(${PROJECT_NAME} PRIVATE -fsanitize=address,undefined)
89108
endif()
90109

91110
# Enable testing if unit tests are enabled

components/mdns/tests/host_unit_test/README.md

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -52,29 +52,38 @@ Repeat with `-DUNIT_TESTS=test_sender` or `-DUNIT_TESTS=test_browse` in a separa
5252

5353
## Fuzzer tests
5454

55-
Build the AFL-instrumented harness (no `-DUNIT_TESTS`; uses `main.c`):
55+
See [fuzzing.md](fuzzing.md) for the AFL++ effectiveness checklist and rationale.
56+
57+
Build separate harnesses for **receive** (parse/query) and **browse** (cache/TXT). Prefer `afl-clang-fast`. Set sanitizer options before fuzzing or reproducing:
5658

5759
```bash
5860
export IDF_PATH=/path/to/esp-idf # required in the fuzz container
61+
export ASAN_OPTIONS="abort_on_error=1:halt_on_error=1:symbolize=0:detect_stack_use_after_return=1:max_malloc_fill_size=$((1<<30))"
62+
export UBSAN_OPTIONS="halt_on_error=1:abort_on_error=1"
5963

6064
cd input && python generate_cases.py && cd ..
6165

62-
cmake -B build2 -S . -G Ninja -DCMAKE_C_COMPILER=afl-cc
63-
cmake --build build2
66+
cmake -B build_fuzz_recv -S . -G Ninja \
67+
-DCMAKE_C_COMPILER=afl-clang-fast -DFUZZ_TARGET=receive
68+
cmake --build build_fuzz_recv
69+
afl-fuzz -i input -o out_recv -- build_fuzz_recv/mdns_host_unit_test
6470

65-
afl-fuzz -i input -o out -- build2/mdns_host_unit_test
71+
cmake -B build_fuzz_browse -S . -G Ninja \
72+
-DCMAKE_C_COMPILER=afl-clang-fast -DFUZZ_TARGET=browse
73+
cmake --build build_fuzz_browse
74+
afl-fuzz -i input -o out_browse -- build_fuzz_browse/mdns_host_unit_test
6675
```
6776

68-
The harness reads packets from stdin and exercises IPv4/IPv6 and port 5353/53 combinations. Crashes are written to `out/default/crashes/`.
77+
Each execution feeds one packet (exact-size copy) into `mdns_packet_push`, with IPv4/IPv6 and port 53/5353 derived from the input. Crashes land under `out_*/default/crashes/`.
6978

7079
### Reproducing a crash
7180

72-
Build the non-unit-test binary with a normal compiler, then pass a crash file:
81+
Build the same `FUZZ_TARGET` with a normal compiler, keep `ASAN_OPTIONS` set, then pass the crash file:
7382

7483
```bash
75-
cmake -B build2 -S .
76-
cmake --build build2
77-
./build2/mdns_host_unit_test out/default/crashes/id_000000,...
84+
cmake -B build_repro -S . -G Ninja -DFUZZ_TARGET=receive
85+
cmake --build build_repro
86+
./build_repro/mdns_host_unit_test out_recv/default/crashes/id_000000,...
7887
```
7988

8089
With sanitizers enabled, ASan/UBSan report buffer overruns and undefined behaviour directly during unit tests and fuzzing.
Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
# mDNS AFL++ fuzzing notes
2+
3+
Checklist from `fuzzing_info.txt` (AFL++ effectiveness tips), applied to
4+
`components/mdns/tests/host_unit_test`. Each item states what we do here and why.
5+
6+
Preferred compiler: `afl-clang-fast` / `afl-cc` (LLVM persistent + deferred forkserver).
7+
8+
Required ASan env when running `afl-fuzz` or reproducing crashes:
9+
10+
```bash
11+
export ASAN_OPTIONS="abort_on_error=1:halt_on_error=1:symbolize=0:detect_stack_use_after_return=1:max_malloc_fill_size=$((1<<30))"
12+
export UBSAN_OPTIONS="halt_on_error=1:abort_on_error=1"
13+
```
14+
15+
## (1) Configure sanitizers and assertions
16+
17+
CMake already links the fuzz binary with `-fsanitize=address,undefined`, `-g3`, and frame pointers. That matches the tip’s default of ASan+UBSan for C targets. Runtime sensitivity (abort-on-error, use-after-return, full malloc fill) must be set via `ASAN_OPTIONS` / `UBSAN_OPTIONS` as above—AFL++ will refuse to run if abort/halt are missing when ASan is linked. We do not enable `_GLIBCXX_DEBUG` because this harness is pure C.
18+
19+
## (2) Prefer the persistent mode
20+
21+
`main.c` uses `__AFL_LOOP(10000)` with deferred init (`__AFL_FUZZ_INIT` / `__AFL_INIT`) and the shared-memory testcase buffer when built with `afl-clang-fast`. That avoids re-execing the process per input and is roughly an order of magnitude faster than plain fork+exec. Crash reproduction builds (normal `cc`) skip these macros and take a crash file path instead.
22+
23+
## (3) Include source files, not header files
24+
25+
Not adopted for this tree. The host harness already has a deliberate multi-file CMake build (mdns sources + Linux stubs + generated `sdkconfig` headers), and pulling `.c` files into one TU would fight IDF’s include layout without unlocking static-only APIs we need—the public receive entry is already `mdns_packet_push``mdns_priv_receive_action`. Keep the existing build; use stubs for symbol substitution instead of `#include "foo.c"`.
26+
27+
## (4) Isolate fuzz tests from each other
28+
29+
Persistent mode reuses process-global mdns state (responder, queries/browsers, cache). After each input we call `mdns_priv_cache_clear()` so cache entries from a previous packet cannot change coverage or hide bugs on the next one. Full deinit/init every iteration would be stronger isolation but much slower; AFL still recycles the process periodically, which resets remaining globals.
30+
31+
## (5) Do not test directly on the fuzz test buffer
32+
33+
The harness `realloc`s an exact-size `input_copy` and `memcpy`s the AFL buffer into it before calling `send_packet`. That way ASan can catch reads past the logical length—AFL’s SHM backing store is larger than `len`, so passing `buf` straight through would miss overflows. The networking stub also copies into a `malloc(len)` packet payload; the harness copy is intentional defense in depth for tip (5).
34+
35+
## (6) Don’t bother freeing memory
36+
37+
Partially followed. We do not free the persistent `input_copy` between iterations (only grow via `realloc`), and we avoid noisy per-packet teardown. We *do* clear the mdns cache each iteration (see (4)) because unbounded cache growth would OOM or make crashes unreproducible—tip (6)’s “skip destructors” advice loses to isolation for this stateful protocol stack.
38+
39+
## (7) Use a memory file descriptor to back named paths
40+
41+
Not applicable. The fuzz surface is an in-memory API (`mdns_packet_push(addr, port, if, data, len)`), not a pathname/`open` interface. No `memfd_create` / `/proc/self/fd/N` wrapper is required.
42+
43+
## (8) Configure the target for smaller buffers
44+
45+
Not changed in production headers. Incoming fuzz length is already capped near `MDNS_MAX_PACKET_SIZE` (1460), which matches the realistic UDP/mDNS bound, so shrinking that further mostly invents artificial TX-path limits rather than exposing RX bugs faster. Name/TXT limits (`MDNS_NAME_MAX_LEN`, `MDNS_TXT_MAX_LEN`) could be reduced under a future fuzz-only compile switch if corpus depth stalls on those paths; document-only for now.
46+
47+
---
48+
49+
## Harness split: `receive` vs `browse`
50+
51+
Unit tests already isolate receiver vs browser with different mocks. Fuzzing benefits from the same split so AFL’s coverage map is not dominated by whichever path mutates first:
52+
53+
| `-DFUZZ_TARGET=` | Context | What it stresses |
54+
|------------------|---------|------------------|
55+
| `receive` (default) | Responder + async queries | Parse, questions/answers, query matching |
56+
| `browse` | Responder + `mdns_browse_new` | Browse/cache/TXT comparison path |
57+
58+
Build separate output dirs and run two `afl-fuzz` instances (or one after the other). Each input still hits a single IPv4/IPv6 × port 53/5353 combo derived from a XOR of the packet bytes (full DNS payload preserved for the seed corpus).
59+
60+
```bash
61+
export ASAN_OPTIONS="abort_on_error=1:halt_on_error=1:symbolize=0:detect_stack_use_after_return=1:max_malloc_fill_size=$((1<<30))"
62+
export UBSAN_OPTIONS="halt_on_error=1:abort_on_error=1"
63+
64+
cd input && python generate_cases.py && cd ..
65+
66+
cmake -B build_fuzz_recv -S . -G Ninja \
67+
-DCMAKE_C_COMPILER=afl-clang-fast -DFUZZ_TARGET=receive
68+
cmake --build build_fuzz_recv
69+
afl-fuzz -i input -o out_recv -- build_fuzz_recv/mdns_host_unit_test
70+
71+
cmake -B build_fuzz_browse -S . -G Ninja \
72+
-DCMAKE_C_COMPILER=afl-clang-fast -DFUZZ_TARGET=browse
73+
cmake --build build_fuzz_browse
74+
afl-fuzz -i input -o out_browse -- build_fuzz_browse/mdns_host_unit_test
75+
```

0 commit comments

Comments
 (0)