From fe098e1f4f17c8aa6ab32bd697e45f335f3e00e1 Mon Sep 17 00:00:00 2001 From: Diebbo Date: Fri, 20 Dec 2024 16:10:51 +0100 Subject: [PATCH 1/2] FIX: CVE vulnerabilities --- modules/pico_dns_common.c | 15 +++++++++++++++ modules/pico_mdns.c | 6 ++++++ 2 files changed, 21 insertions(+) diff --git a/modules/pico_dns_common.c b/modules/pico_dns_common.c index ab577990..54b7dfbf 100644 --- a/modules/pico_dns_common.c +++ b/modules/pico_dns_common.c @@ -112,6 +112,7 @@ pico_dns_namelen_comp( char *name ) char * pico_dns_decompress_name( char *name, pico_dns_packet *packet ) { + uint16_t packet_len = sizeof(pico_dns_packet); char decompressed_name[PICO_DNS_NAMEBUF_SIZE] = { 0 }; @@ -120,9 +121,17 @@ pico_dns_decompress_name( char *name, pico_dns_packet *packet ) uint16_t decompressed_index = 0; char *label = NULL, *next = NULL; + if (!name || !packet) { + pico_err = PICO_ERR_EINVAL; + return NULL; + } + /* Reading labels until reaching to pointer or NULL terminator. * Only one pointer is allowed in DNS compression, the pointer is always the last according to the RFC */ dns_name_foreach_label_safe(label, name, next, PICO_DNS_NAMEBUF_SIZE) { + if (!label || (*label & 0xFF) >= PICO_DNS_NAMEBUF_SIZE) { + return NULL; + } uint8_t label_size = (uint8_t)(*label+1); if (decompressed_index + label_size >= PICO_DNS_NAMEBUF_SIZE) { @@ -140,6 +149,12 @@ pico_dns_decompress_name( char *name, pico_dns_packet *packet ) /* Found compression bits */ ptr = (uint16_t)((((uint16_t) *label) & 0x003F) << 8); ptr = (uint16_t)(ptr | (uint16_t) *(label + 1)); + + /* Check if the pointer is within the packet */ + if (ptr >= packet_len) { + return NULL; + } + label = (char *)((uint8_t *)packet + ptr); dns_name_foreach_label_safe(label, label, next, PICO_DNS_NAMEBUF_SIZE-decompressed_index) { diff --git a/modules/pico_mdns.c b/modules/pico_mdns.c index 3e713e15..c843bdb7 100644 --- a/modules/pico_mdns.c +++ b/modules/pico_mdns.c @@ -2200,6 +2200,12 @@ pico_mdns_handle_data_as_answers_generic(struct pico_stack *S, return -1; } + // check that the number of answers/responses correspond to the number of questions + if (count != pico_tree_count(&S->MDNSOwnRecords)) { + mdns_dbg("Number of answers does not match the number of questions\n"); + return -1; + } + /* TODO: When receiving multiple authoritative answers, */ /* they should be sorted in lexicographical order */ /* (just like in pico_mdns_record_am_i_lexi_later) */ From f22d7ddb058e571b811d81efaa6a12ab2a3fbb25 Mon Sep 17 00:00:00 2001 From: scribam Date: Sat, 29 Nov 2025 18:31:28 +0100 Subject: [PATCH 2/2] Fix unit tests --- test/unit/modunit_pico_dns_common.c | 22 +++++++++++++++++++++- test/unit/modunit_pico_mdns.c | 18 ++++++++++++------ 2 files changed, 33 insertions(+), 7 deletions(-) diff --git a/test/unit/modunit_pico_dns_common.c b/test/unit/modunit_pico_dns_common.c index ff001e4b..60a631e1 100644 --- a/test/unit/modunit_pico_dns_common.c +++ b/test/unit/modunit_pico_dns_common.c @@ -374,7 +374,7 @@ START_TEST(tc_pico_dns_fill_packet_header) /* MARK: dns_fill_packet_header */ fail_unless(0 == memcmp((void *)header, (void *)query_buf, 12), "Comparing query header failed!\n"); - /* Create a answer header */ + /* Create an answer header */ pico_dns_fill_packet_header(header, 0, 1, 1, 1); fail_unless(0 == memcmp((void *)header, (void *)answer_buf, 12), @@ -1042,10 +1042,30 @@ START_TEST(tc_pico_dns_decompress_name) /* MARK: dns_decompress_name */ char name[] = "\4mail\xc0\x02"; char name2[] = "\xc0\x02"; char buf[] = "00\6google\3com"; + char empty_name[] = ""; char *ret; printf("*********************** starting %s * \n", __func__); + /* NULL name */ + ret = pico_dns_decompress_name(NULL, (pico_dns_packet *)buf); + fail_unless(ret == NULL, "pico_dns_decompress_name(NULL, ...) should be invalid\n"); + + /* NULL buf */ + ret = pico_dns_decompress_name(name, NULL); + fail_unless(ret == NULL, "pico_dns_decompress_name(..., NULL) should be invalid\n"); + + /* Empty name */ + ret = pico_dns_decompress_name(empty_name, (pico_dns_packet *)buf); + + /* Fail conditions */ + fail_unless(ret != NULL, "Name ptr returned is NULL"); + fail_unless(strcmp(ret, "") == 0, "Not correctly decompressed: '%s'!\n", ret); + + /* Free memory */ + PICO_FREE(ret); + ret = NULL; + /* Test normal DNS name compression */ ret = pico_dns_decompress_name(name, (pico_dns_packet *)buf); diff --git a/test/unit/modunit_pico_mdns.c b/test/unit/modunit_pico_mdns.c index e56e8fb9..402d7ed8 100644 --- a/test/unit/modunit_pico_mdns.c +++ b/test/unit/modunit_pico_mdns.c @@ -1584,7 +1584,6 @@ START_TEST(tc_mdns_handle_data_as_questions) /* MARK: handle_data_as_questions * }; uint16_t len = 0; uint8_t *ptr = NULL; - int ret = 0; struct pico_dns_question *a = NULL, *b = NULL; struct pico_mdns_record *record1 = NULL, *record2 = NULL; struct pico_stack *S = NULL; @@ -1598,7 +1597,6 @@ START_TEST(tc_mdns_handle_data_as_questions) /* MARK: handle_data_as_questions * PICO_MDNS_QUESTION_FLAG_UNICAST_RES, 0); fail_if(!a, "dns_question_create failed!\n"); pico_tree_insert(&qtree, a); - fail_unless(ret == 0, "dns_question_vector_add returned error!\n"); b = pico_mdns_question_create(qurl2, &len, PICO_PROTO_IPV4, PICO_DNS_TYPE_A, 0, 0); fail_if(!b, "dns_question_create failed!\n"); @@ -1620,7 +1618,8 @@ START_TEST(tc_mdns_handle_data_as_questions) /* MARK: handle_data_as_questions * pico_tree_insert(&S->MDNSOwnRecords, record1); pico_tree_insert(&S->MDNSOwnRecords, record2); - ptr = ((uint8_t *)packet + 12); + /* Move past the DNS packet header */ + ptr = ((uint8_t *)packet + sizeof(struct pico_dns_header)); antree = pico_mdns_handle_data_as_questions(S, &ptr, 2, packet); fail_unless(2 == pico_tree_count(&antree), @@ -1656,12 +1655,15 @@ START_TEST(tc_mdns_handle_data_as_answers) /* MARK: handle_data_as_answers */ fail_if(!a, "dns_record_create returned NULL!\n"); pico_tree_insert(&rtree, a->record); pico_tree_insert(&rtree, b->record); + pico_tree_insert(&S->MDNSOwnRecords, a); + pico_tree_insert(&S->MDNSOwnRecords, b); /* Try to create an answer packet */ packet = pico_dns_answer_create(&rtree, NULL, NULL, &len); fail_if (packet == NULL, "mdns_answer_create returned NULL!\n"); - ptr = ((uint8_t *)packet + 12); + /* Move past the DNS packet header */ + ptr = ((uint8_t *)packet + sizeof(struct pico_dns_header)); ret = pico_mdns_handle_data_as_answers_generic(S, &ptr, 2, packet, 0); fail_unless(0 == ret, "mdns_handle_data_as_answers failed!\n"); @@ -1696,12 +1698,15 @@ START_TEST(tc_mdns_handle_data_as_authorities) /* MARK: handle_data_as_authoriti fail_if(!a, "dns_record_create returned NULL!\n"); pico_tree_insert(&rtree, a->record); pico_tree_insert(&rtree, b->record); + pico_tree_insert(&S->MDNSOwnRecords, a); + pico_tree_insert(&S->MDNSOwnRecords, b); /* Try to create an answer packet */ packet = pico_dns_answer_create(&rtree, NULL, NULL, &len); fail_if (packet == NULL, "mdns_answer_create returned NULL!\n"); - ptr = ((uint8_t *)packet + 12); + /* Move past the DNS packet header */ + ptr = ((uint8_t *)packet + sizeof(struct pico_dns_header)); ret = pico_mdns_handle_data_as_answers_generic(S, &ptr, 2, packet, 1); fail_unless(0 == ret, "mdns_handle_data_as_answers failed!\n"); @@ -1837,7 +1842,8 @@ START_TEST(tc_mdns_apply_known_answer_suppression) /* MARK: apply_k_a_s */ packet = pico_dns_answer_create(&antree, NULL, NULL, &len); fail_if (packet == NULL, "mdns_answer_create returned NULL!\n"); - ptr = ((uint8_t *)packet + 12); + /* Move past the DNS packet header */ + ptr = ((uint8_t *)packet + sizeof(struct pico_dns_header)); printf("Applying Known answer suppression...\n");