From 4041654f119280bee41906970dc904f2a7270dcc Mon Sep 17 00:00:00 2001 From: Dustin Spicuzza Date: Mon, 24 Aug 2026 05:11:51 +0000 Subject: [PATCH 1/2] Add cmocka mDNS parsing test --- .github/workflows/ci.yml | 2 +- .gitignore | 1 + CMakeLists.txt | 13 +++++ tests/CMakeLists.txt | 15 ++++++ tests/mdns_parse_test.c | 100 +++++++++++++++++++++++++++++++++++++++ 5 files changed, 130 insertions(+), 1 deletion(-) create mode 100644 tests/CMakeLists.txt create mode 100644 tests/mdns_parse_test.c diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5377ce5..1a5266a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -25,7 +25,7 @@ jobs: uses: actions/checkout@v3 - name: Configure - run: cmake -B _build -DCMAKE_BUILD_TYPE=${{ matrix.build_type }} + run: cmake -B _build -DCMAKE_BUILD_TYPE=${{ matrix.build_type }} -DMDNS_BUILD_TESTS=ON - name: Build run: cmake --build _build --config ${{ matrix.build_type }} diff --git a/.gitignore b/.gitignore index 343c19a..9fffa48 100644 --- a/.gitignore +++ b/.gitignore @@ -189,6 +189,7 @@ pip-log.txt ############### #Project builds +/_build/ lib/** bin/** dist/** diff --git a/CMakeLists.txt b/CMakeLists.txt index 50f1f05..a44cffa 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -2,6 +2,7 @@ cmake_minimum_required(VERSION 3.5) project(mdns VERSION 1.4.2 LANGUAGES C) option(MDNS_BUILD_EXAMPLE "build example" ON) +option(MDNS_BUILD_TESTS "build tests" OFF) # Set the output of the libraries and executables. set(EXECUTABLE_OUTPUT_PATH ${PROJECT_BINARY_DIR}/bin) @@ -32,6 +33,18 @@ if(MDNS_BUILD_EXAMPLE) target_link_libraries(${PROJECT_NAME}_example ${PROJECT_NAME}) endif() +# ############################################################################## +# tests +# ############################################################################## + +if(MDNS_BUILD_TESTS) + if(${CMAKE_VERSION} VERSION_LESS 3.14) + message(FATAL_ERROR "MDNS_BUILD_TESTS requires CMake 3.14 or newer") + endif() + enable_testing() + add_subdirectory(tests) +endif() + # ############################################################################## # install # ############################################################################## diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt new file mode 100644 index 0000000..1c9a757 --- /dev/null +++ b/tests/CMakeLists.txt @@ -0,0 +1,15 @@ +include(FetchContent) + +set(BUILD_SHARED_LIBS OFF CACHE BOOL "Build static libraries" FORCE) +set(UNIT_TESTING OFF CACHE BOOL "Build cmocka unit tests" FORCE) +set(WITH_EXAMPLES OFF CACHE BOOL "Build cmocka examples" FORCE) + +FetchContent_Declare( + cmocka + URL https://cmocka.org/files/1.1/cmocka-1.1.7.tar.xz + URL_HASH SHA256=810570eb0b8d64804331f82b29ff47c790ce9cd6b163e98d47a4807047ecad82) +FetchContent_MakeAvailable(cmocka) + +add_executable(mdns_parse_test mdns_parse_test.c) +target_link_libraries(mdns_parse_test PRIVATE mdns::mdns cmocka::cmocka) +add_test(NAME mdns_parse COMMAND mdns_parse_test) diff --git a/tests/mdns_parse_test.c b/tests/mdns_parse_test.c new file mode 100644 index 0000000..1fdf94e --- /dev/null +++ b/tests/mdns_parse_test.c @@ -0,0 +1,100 @@ +#include +#include +#include +#include +#include + +#include + +#include "mdns.h" + +struct parsed_record { + int callback_count; + mdns_entry_type_t entry; + uint16_t query_id; + uint16_t type; + uint16_t rclass; + uint32_t ttl; + char name[64]; + size_t name_length; + struct sockaddr_in address; + int address_parsed; +}; + +static int +capture_record(int sock, const struct sockaddr* from, size_t addrlen, mdns_entry_type_t entry, + uint16_t query_id, uint16_t rtype, uint16_t rclass, uint32_t ttl, const void* data, + size_t size, size_t name_offset, size_t name_length, size_t record_offset, + size_t record_length, void* user_data) { + struct parsed_record* record = user_data; + mdns_string_t name; + + (void)sock; + (void)from; + (void)addrlen; + (void)name_length; + + ++record->callback_count; + record->entry = entry; + record->query_id = query_id; + record->type = rtype; + record->rclass = rclass; + record->ttl = ttl; + name = mdns_string_extract(data, size, &name_offset, record->name, sizeof(record->name)); + record->name_length = name.length; + record->address_parsed = + mdns_record_parse_a(data, size, record_offset, record_length, &record->address) != NULL; + return 0; +} + +static void +test_parses_simple_a_response(void** state) { + static const uint8_t packet[] = { + 0x00, 0x00, /* Query ID */ + 0x84, 0x00, /* Response and authoritative-answer flags */ + 0x00, 0x00, /* Questions */ + 0x00, 0x01, /* Answer records */ + 0x00, 0x00, /* Authority records */ + 0x00, 0x00, /* Additional records */ + 0x04, 'h', 'o', 's', 't', 0x05, 'l', 'o', 'c', 'a', 'l', 0x00, /* host.local. */ + 0x00, 0x01, /* A record */ + 0x80, 0x01, /* Cache flush and class IN */ + 0x00, 0x00, 0x00, 0x78, /* TTL: 120 seconds */ + 0x00, 0x04, /* Record data length */ + 0xc0, 0xa8, 0x01, 0x2a, /* 192.168.1.42 */ + }; + static const uint8_t expected_address[] = {0xc0, 0xa8, 0x01, 0x2a}; + struct parsed_record record = {0}; + size_t offset = 12; + uint16_t query_id = mdns_ntohs(packet); + size_t answer_count = mdns_ntohs(packet + 6); + size_t parsed; + + (void)state; + + parsed = mdns_records_parse(-1, NULL, 0, packet, sizeof(packet), &offset, MDNS_ENTRYTYPE_ANSWER, + query_id, answer_count, capture_record, &record); + + assert_int_equal(parsed, 1); + assert_int_equal(offset, sizeof(packet)); + assert_int_equal(record.callback_count, 1); + assert_int_equal(record.entry, MDNS_ENTRYTYPE_ANSWER); + assert_int_equal(record.query_id, 0); + assert_int_equal(record.type, MDNS_RECORDTYPE_A); + assert_int_equal(record.rclass, MDNS_CACHE_FLUSH | MDNS_CLASS_IN); + assert_int_equal(record.ttl, 120); + assert_int_equal(record.name_length, strlen("host.local.")); + assert_memory_equal(record.name, "host.local.", record.name_length); + assert_true(record.address_parsed); + assert_int_equal(record.address.sin_family, AF_INET); + assert_memory_equal(&record.address.sin_addr, expected_address, sizeof(expected_address)); +} + +int +main(void) { + const struct CMUnitTest tests[] = { + cmocka_unit_test(test_parses_simple_a_response), + }; + + return cmocka_run_group_tests(tests, NULL, NULL); +} From cf1f6479679e4a45e777fbf8f241fb6e5905807c Mon Sep 17 00:00:00 2001 From: Dustin Spicuzza Date: Mon, 31 Aug 2026 07:02:46 +0000 Subject: [PATCH 2/2] Validate record parser RDATA boundaries Fixed-record range checks could wrap when offset and length were added, while PTR and SRV names could consume bytes beyond their declared RDATA. Use subtraction-based bounds checks, keep encoded names within RDLENGTH, and accept the valid root-name SRV representation. --- mdns.h | 36 +++++++++++++++----- tests/mdns_parse_test.c | 74 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 101 insertions(+), 9 deletions(-) diff --git a/mdns.h b/mdns.h index d0794bf..eda690f 100644 --- a/mdns.h +++ b/mdns.h @@ -1500,14 +1500,30 @@ mdns_goodbye_multicast(int sock, void* buffer, size_t capacity, mdns_record_t an MDNS_CLASS_IN, 0); } +static inline int +mdns_record_range_valid(size_t size, size_t offset, size_t length) { + return (offset <= size) && (length <= (size - offset)); +} + +static inline mdns_string_t +mdns_record_parse_name(const void* buffer, size_t size, size_t offset, size_t length, + char* strbuffer, size_t capacity) { + mdns_string_t empty = {0, 0}; + if (!length || !mdns_record_range_valid(size, offset, length)) + return empty; + + size_t name_offset = offset; + mdns_string_t name = mdns_string_extract(buffer, size, &name_offset, strbuffer, capacity); + if ((name_offset == offset) || (name_offset > (offset + length))) + return empty; + return name; +} + static inline mdns_string_t mdns_record_parse_ptr(const void* buffer, size_t size, size_t offset, size_t length, char* strbuffer, size_t capacity) { // PTR record is just a string - if ((size >= offset + length) && (length >= 2)) - return mdns_string_extract(buffer, size, &offset, strbuffer, capacity); - mdns_string_t empty = {0, 0}; - return empty; + return mdns_record_parse_name(buffer, size, offset, length, strbuffer, capacity); } static inline mdns_record_srv_t @@ -1520,14 +1536,16 @@ mdns_record_parse_srv(const void* buffer, size_t size, size_t offset, size_t len // 2 bytes network-order unsigned priority // 2 bytes network-order unsigned weight // 2 bytes network-order unsigned port - // string: discovery (domain) name, minimum 2 bytes when compressed - if ((size >= offset + length) && (length >= 8)) { + // string: discovery (domain) name, minimum 1 byte for the root name + if ((length >= 7) && mdns_record_range_valid(size, offset, length)) { const uint16_t* recorddata = (const uint16_t*)MDNS_POINTER_OFFSET_CONST(buffer, offset); srv.priority = mdns_ntohs(recorddata++); srv.weight = mdns_ntohs(recorddata++); srv.port = mdns_ntohs(recorddata++); offset += 6; - srv.name = mdns_string_extract(buffer, size, &offset, strbuffer, capacity); + srv.name = mdns_record_parse_name(buffer, size, offset, length - 6, strbuffer, capacity); + if (!srv.name.str) + memset(&srv, 0, sizeof(mdns_record_srv_t)); } return srv; } @@ -1540,7 +1558,7 @@ mdns_record_parse_a(const void* buffer, size_t size, size_t offset, size_t lengt #ifdef __APPLE__ addr->sin_len = sizeof(struct sockaddr_in); #endif - if ((size >= offset + length) && (length == 4)) + if ((length == 4) && mdns_record_range_valid(size, offset, length)) memcpy(&addr->sin_addr.s_addr, MDNS_POINTER_OFFSET(buffer, offset), 4); return addr; } @@ -1553,7 +1571,7 @@ mdns_record_parse_aaaa(const void* buffer, size_t size, size_t offset, size_t le #ifdef __APPLE__ addr->sin6_len = sizeof(struct sockaddr_in6); #endif - if ((size >= offset + length) && (length == 16)) + if ((length == 16) && mdns_record_range_valid(size, offset, length)) memcpy(&addr->sin6_addr, MDNS_POINTER_OFFSET(buffer, offset), 16); return addr; } diff --git a/tests/mdns_parse_test.c b/tests/mdns_parse_test.c index 1fdf94e..807f70f 100644 --- a/tests/mdns_parse_test.c +++ b/tests/mdns_parse_test.c @@ -90,10 +90,84 @@ test_parses_simple_a_response(void** state) { assert_memory_equal(&record.address.sin_addr, expected_address, sizeof(expected_address)); } +static void +test_record_parsers_reject_wrapped_ranges(void** state) { + uint8_t a_data[4] = {1, 2, 3, 4}; + uint8_t aaaa_data[16] = {1, 2, 3, 4}; + uint8_t srv_data[8] = {0, 1, 0, 2, 0, 3, 0}; + struct sockaddr_in address; + struct sockaddr_in6 address6; + char name_buffer[32]; + mdns_record_srv_t srv; + + (void)state; + mdns_record_parse_a(a_data, 1, SIZE_MAX - 3, 4, &address); + mdns_record_parse_aaaa(aaaa_data, 1, SIZE_MAX - 15, 16, &address6); + srv = mdns_record_parse_srv(srv_data, 1, SIZE_MAX - 7, 8, name_buffer, + sizeof(name_buffer)); + assert_int_equal(address.sin_addr.s_addr, 0); + assert_memory_equal(&address6.sin6_addr, (uint8_t[16]){0}, 16); + assert_int_equal(srv.priority, 0); + assert_int_equal(srv.weight, 0); + assert_int_equal(srv.port, 0); +} + +static void +test_record_names_stay_within_rdata(void** state) { + static const uint8_t ptr_data[] = {3, 'b', 'a', 'd', 0}; + static const uint8_t srv_data[] = {0, 1, 0, 2, 0, 3, 3, 'b', 'a', 'd', 0}; + char ptr_buffer[32] = {0}; + char srv_buffer[32] = {0}; + mdns_string_t ptr; + mdns_record_srv_t srv; + + (void)state; + ptr = mdns_record_parse_ptr(ptr_data, sizeof(ptr_data), 0, 2, ptr_buffer, + sizeof(ptr_buffer)); + srv = mdns_record_parse_srv(srv_data, sizeof(srv_data), 0, 8, srv_buffer, + sizeof(srv_buffer)); + assert_int_equal(ptr.length, 0); + assert_int_equal(srv.priority, 0); + assert_int_equal(srv.weight, 0); + assert_int_equal(srv.port, 0); + assert_int_equal(srv.name.length, 0); +} + +static void +test_srv_accepts_root_target(void** state) { + static const uint8_t data[] = {0, 1, 0, 2, 0x13, 0x37, 0}; + char name_buffer[32]; + mdns_record_srv_t srv; + + (void)state; + srv = mdns_record_parse_srv(data, sizeof(data), 0, sizeof(data), name_buffer, + sizeof(name_buffer)); + assert_int_equal(srv.priority, 1); + assert_int_equal(srv.weight, 2); + assert_int_equal(srv.port, 0x1337); + assert_int_equal(srv.name.length, 0); +} + +static void +test_ptr_allows_compression_target_outside_rdata(void** state) { + static const uint8_t data[] = {3, 'f', 'o', 'o', 0, 0xc0, 0}; + char name_buffer[32] = {0}; + mdns_string_t name; + + (void)state; + name = mdns_record_parse_ptr(data, sizeof(data), 5, 2, name_buffer, sizeof(name_buffer)); + assert_int_equal(name.length, 4); + assert_memory_equal(name.str, "foo.", 4); +} + int main(void) { const struct CMUnitTest tests[] = { cmocka_unit_test(test_parses_simple_a_response), + cmocka_unit_test(test_record_parsers_reject_wrapped_ranges), + cmocka_unit_test(test_record_names_stay_within_rdata), + cmocka_unit_test(test_srv_accepts_root_target), + cmocka_unit_test(test_ptr_allows_compression_target_outside_rdata), }; return cmocka_run_group_tests(tests, NULL, NULL);