From a8ed0dec571995e958e195787b87c49ac13d0797 Mon Sep 17 00:00:00 2001 From: Diebbo Date: Sat, 29 Nov 2025 15:54:49 +0100 Subject: [PATCH 1/3] FIX: adjusting ptr checks --- modules/pico_dns_client.c | 8 +++++--- modules/pico_dns_sd.c | 17 +++++++++++++++++ modules/pico_dns_sd.h | 5 +++++ 3 files changed, 27 insertions(+), 3 deletions(-) diff --git a/modules/pico_dns_client.c b/modules/pico_dns_client.c index d220fcb8..ee258095 100644 --- a/modules/pico_dns_client.c +++ b/modules/pico_dns_client.c @@ -596,9 +596,6 @@ static void pico_dns_client_callback(uint16_t ev, struct pico_socket *s) } header = (struct pico_dns_header *)dns_response; - domain = (char *)header + sizeof(struct pico_dns_header); - qsuffix = (struct pico_dns_question_suffix *)pico_dns_client_seek(domain); - /* valid asuffix is determined dynamically later on */ if (pico_dns_client_check_header(header) < 0) return; @@ -607,6 +604,11 @@ static void pico_dns_client_callback(uint16_t ev, struct pico_socket *s) if (!q) return; + // FIX: what if the query is not a PTR query? + domain = (char *)header + sizeof(struct pico_dns_header); + qsuffix = (struct pico_dns_question_suffix *)pico_dns_client_seek(domain); + /* valid asuffix is determined dynamically later on */ + if (pico_dns_client_check_qsuffix(qsuffix, q) < 0) return; diff --git a/modules/pico_dns_sd.c b/modules/pico_dns_sd.c index 1cd6210e..5080c040 100644 --- a/modules/pico_dns_sd.c +++ b/modules/pico_dns_sd.c @@ -73,9 +73,26 @@ pico_dns_sd_kv_vector_strlen( kv_vector *vector ) /* Iterate over the key-value pairs */ for (i = 0; i < vector->count; i++) { iterator = pico_dns_sd_kv_vector_get(vector, i); + + if (!iterator || !iterator->key) { + pico_err = PICO_ERR_EINVAL; + return 0; + } + + if (len + 1u + strlen(iterator->key) > PICO_DNS_SD_KV_MAXLEN) { + pico_err = PICO_ERR_EINVAL; + return 0; + } + len = (uint16_t) (len + 1u + /* Length byte */ strlen(iterator->key) /* Length of the key */); + if (iterator->value) { + if (len + 1u + strlen(iterator->value) > PICO_DNS_SD_KV_MAXLEN) { + pico_err = PICO_ERR_EINVAL; + return 0; + } + len = (uint16_t) (len + 1u /* '=' char */ + strlen(iterator->value) /* Length of value */); } diff --git a/modules/pico_dns_sd.h b/modules/pico_dns_sd.h index 0da5cf53..ecff8b3b 100644 --- a/modules/pico_dns_sd.h +++ b/modules/pico_dns_sd.h @@ -44,6 +44,11 @@ typedef struct #define PICO_DNS_SD_KV_VECTOR_DECLARE(name) \ kv_vector (name) = {0} +/* **************************************************************************** + * Maximum length of a key-value pair. + * ****************************************************************************/ +#define PICO_DNS_SD_KV_MAXLEN (0xFFFFu) + /* **************************************************************************** * Just calls pico_mdns_init in it's turn to initialise the mDNS-module. * See pico_mdns.h for description. From cd09f3708f607d4bfbd21a4cd7736fed1582e904 Mon Sep 17 00:00:00 2001 From: scribam Date: Sat, 29 Nov 2025 16:40:33 +0100 Subject: [PATCH 2/3] Check if a vector->pairs is not NULL before to use its value --- modules/pico_dns_sd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/pico_dns_sd.c b/modules/pico_dns_sd.c index 5080c040..2d149f49 100644 --- a/modules/pico_dns_sd.c +++ b/modules/pico_dns_sd.c @@ -575,7 +575,7 @@ pico_dns_sd_kv_vector_get( kv_vector *vector, uint16_t index ) return NULL; /* Return record with conditioned index */ - if (index < vector->count) + if (vector->pairs && (index < vector->count)) return vector->pairs[index]; return NULL; From 005544ae354b68d6b58e70972fe0fcfb39134a48 Mon Sep 17 00:00:00 2001 From: scribam Date: Sat, 29 Nov 2025 17:21:20 +0100 Subject: [PATCH 3/3] Add unit tests for pico_dns_sd_kv_vector_strlen and pico_dns_sd_txt_record_create --- test/unit/modunit_pico_dns_sd.c | 78 +++++++++++++++++++++++++++++++++ 1 file changed, 78 insertions(+) diff --git a/test/unit/modunit_pico_dns_sd.c b/test/unit/modunit_pico_dns_sd.c index a0261fda..c3ce8865 100644 --- a/test/unit/modunit_pico_dns_sd.c +++ b/test/unit/modunit_pico_dns_sd.c @@ -68,6 +68,54 @@ START_TEST(tc_dns_sd_kv_vector_strlen) kv_vector pairs = { 0 }; + uint16_t len; + char *long_key = NULL; + char *long_value = NULL; + + /* NULL vector */ + len = pico_dns_sd_kv_vector_strlen(NULL); + fail_unless(len == 0, "dns_sd_kv_vector_strlen returned wrong length for a NULL vector!\n"); + + /* Empty vector */ + len = pico_dns_sd_kv_vector_strlen(&pairs); + fail_unless(len == 0, "dns_sd_kv_vector_strlen returned wrong length for empty vector!\n"); + + /* Wrong vector count with empty vector */ + pairs.count = 1; + len = pico_dns_sd_kv_vector_strlen(&pairs); + fail_unless(len == 0, "dns_sd_kv_vector_strlen returned wrong length for a vector with a wrong count!\n"); + pairs.count = 0; + + /* Wrong vector key */ + pico_dns_sd_kv_vector_add(&pairs, text, value); + pairs.pairs[0]->key = NULL; + len = pico_dns_sd_kv_vector_strlen(&pairs); + fail_unless(len == 0, "dns_sd_kv_vector_strlen returned wrong length for a vector with a wrong key!\n"); + pico_dns_sd_kv_vector_erase(&pairs); + + /* Oversize key */ + long_key = malloc(PICO_DNS_SD_KV_MAXLEN + 1); + for (uint16_t i = 0; i < PICO_DNS_SD_KV_MAXLEN; i++) { + long_key[i] = '1'; + } + long_key[PICO_DNS_SD_KV_MAXLEN] = '\0'; + pico_dns_sd_kv_vector_add(&pairs, long_key, value); + len = pico_dns_sd_kv_vector_strlen(&pairs); + fail_unless(len == 0, "dns_sd_kv_vector_strlen returned wrong length for a vector with an oversize key!\n"); + pico_dns_sd_kv_vector_erase(&pairs); + free(long_key); + + /* Oversize value */ + long_value = malloc(PICO_DNS_SD_KV_MAXLEN + 1); + for (uint16_t i = 0; i < PICO_DNS_SD_KV_MAXLEN; i++) { + long_value[i] = '1'; + } + long_value[PICO_DNS_SD_KV_MAXLEN] = '\0'; + pico_dns_sd_kv_vector_add(&pairs, text, long_value); + len = pico_dns_sd_kv_vector_strlen(&pairs); + fail_unless(len == 0, "dns_sd_kv_vector_strlen returned wrong length for a vector with an oversize value!\n"); + pico_dns_sd_kv_vector_erase(&pairs); + free(long_value); pico_dns_sd_kv_vector_add(&pairs, text, value); pico_dns_sd_kv_vector_add(&pairs, text2, NULL); @@ -116,6 +164,7 @@ START_TEST(tc_dns_sd_txt_record_create) { struct pico_mdns_record *record = NULL; struct pico_stack *S = NULL; + char *long_value = NULL; kv_vector pairs = { 0 }; @@ -128,6 +177,35 @@ START_TEST(tc_dns_sd_txt_record_create) pico_stack_init(&S); + /* Empty pair */ + record = pico_dns_sd_txt_record_create(S, "test.local", pairs, 10, + PICO_MDNS_RECORD_UNIQUE); + fail_unless(record == NULL, + "pico_dns_sd_txt_record_create shouldn't have a record!\n"); + + /* Pair with an empty value */ + pico_dns_sd_kv_vector_add(&pairs, text, NULL); + record = pico_dns_sd_txt_record_create(S, "test.local", pairs, 10, + PICO_MDNS_RECORD_UNIQUE); + fail_unless(record != NULL, + "pico_dns_sd_txt_record_create should have a record!\n"); + pico_dns_sd_kv_vector_erase(&pairs); + pico_mdns_record_delete((void **)&record); + + /* Pair with an oversize value */ + long_value = malloc(PICO_DNS_SD_KV_MAXLEN + 1); + for (uint16_t i = 0; i < PICO_DNS_SD_KV_MAXLEN; i++) { + long_value[i] = '1'; + } + long_value[PICO_DNS_SD_KV_MAXLEN] = '\0'; + pico_dns_sd_kv_vector_add(&pairs, text, long_value); + record = pico_dns_sd_txt_record_create(S, "test.local", pairs, 10, + PICO_MDNS_RECORD_UNIQUE); + fail_unless(record == NULL, + "pico_dns_sd_txt_record_create shouldn't have a record!\n"); + pico_dns_sd_kv_vector_erase(&pairs); + free(long_value); + pico_dns_sd_kv_vector_add(&pairs, text, value); pico_dns_sd_kv_vector_add(&pairs, text2, NULL); pico_dns_sd_kv_vector_add(&pairs, text3, value3);