fuzz: harden ndpi_hex_decode and add targeted parser harnesses#3163
Conversation
|
@parasol-aser, could you sign the CLA, please? |
is there a link? |
https://docs.google.com/forms/d/e/1FAIpQLScbtEQSqv88HGN1YFENxiIQoRqvFh_5NL9pqwmJjZmU-wMbSA/viewform |
|
@IvanNardi done |
|
As a first step, I rebased your code to add a fix |
IvanNardi
left a comment
IvanNardi
left a comment
There was a problem hiding this comment.
I have some doubts about the dns and http fuzzers.
The rest seems right.
@parasol-aser, what do you think?
| } | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Please move these unit tests to example/utests.c with the same logic
There was a problem hiding this comment.
Moved checkHexDecode / checkTlsBlocksDecode / rewriteHexCase helpers to example/utests.c (same logic, static).
- fuzz_dns_parse / fuzz_http_parse now call the parsers directly (ndpi_search_dns / ndpi_search_http_tcp) with a synthesised packet_struct instead of driving ndpi_detection_process_packet, matching the fuzz_is_stun / fuzz_quic_get_crypto_data pattern. - Expose ndpi_search_dns and ndpi_search_http_tcp via ndpi_private.h so fuzz/unit targets can reach them; wire -DNDPI_LIB_COMPILATION into the two fuzz target CFLAGS. - Move hexDecodeUnitTest / tlsBlocksUnitTest (and the checkHexDecode, checkTlsBlocksDecode, rewriteHexCase helpers) from tests/unit/unit.c to example/utests.c; register them in run_unit_tests() so they run via ndpiReader --run-tests with the rest of the suite.
|
Pushed 0843f93 addressing the review:
Built + smoke-ran both fuzzers locally ( |
|
IvanNardi
left a comment
IvanNardi
left a comment
There was a problem hiding this comment.
Thank you very much, for the fix and for all the follow-ups
|
@parasol-aser, great work |
|
@IvanNardi happy to contribute, and thank you for your work! |
2 participants
Summary
Closes #3159.
This follows the issue thread's requested fix direction:
ndpi_hex_decode()now uses a bounded nibble decoder instead of relying onsscanf()over a borrowed buffer, rather than papering overndpi_decode_tls_blocks()with a scratch NUL-terminated copy. It also upstreams the directfuzz_ndpi_decode_tls_blocksreproducer that Ivan Nardi asked about in the follow-up comment, plus three additional focused parser fuzz targets from the same fuzzing pass.Changes
sscanf("%02hhX", ...)loop with a bounded hex decoder that accepts explicit-length, non-NUL-terminated buffers, rejects odd-length or malformed hex, and zerosout_lenon failurendpi_decode_tls_blocks()to zeronum_tls_blockson entry and fail closed on malformed decode results instead of partially parsing truncated datandpi_hex_decode()andndpi_decode_tls_blocks()inndpi_api.hfuzz_ndpi_decode_tls_blocksand wire it intofuzz/Makefile.amfuzz_dns_parse,fuzz_http_parse, andfuzz_tls_client_server_helloharnesses and register them infuzz/Makefile.amTest Plan
make -j4 -C fuzz fuzz_ndpi_decode_tls_blocks fuzz_dns_parse fuzz_http_parse fuzz_tls_client_server_hello./fuzz/fuzz_ndpi_decode_tls_blocks -fork=2 -ignore_crashes=1 -max_total_time=20 fuzz/corpus/fuzz_ndpi_decode_tls_blocks/fuzz_dns_parse,fuzz_http_parse, andfuzz_tls_client_server_hellowith-runs=0after staging the expected sidecar data files next to the binariesmake -j4 -C tests/unit, but this environment is missing thejson.h/ json-c headers needed to buildtests/unit/unit