diff --git a/components/esp_dns/esp_dns_doh.c b/components/esp_dns/esp_dns_doh.c index b4e6efc8b..cc0164fb0 100644 --- a/components/esp_dns/esp_dns_doh.c +++ b/components/esp_dns/esp_dns_doh.c @@ -156,10 +156,15 @@ esp_err_t esp_dns_http_event_handler(esp_http_client_event_t *evt) /* Check if the buffer indicates an HTTP error response */ if (HttpStatus_Ok == esp_http_client_get_status_code(evt->client)) { - /* Parse the DNS response */ - esp_dns_parse_response((uint8_t *)handle->response_buffer.buffer, - handle->response_buffer.length, - &handle->response_buffer.dns_response); + if (handle->response_buffer.length >= sizeof(dns_header_t)) { + /* Parse the DNS response */ + esp_dns_parse_response((uint8_t *)handle->response_buffer.buffer, + handle->response_buffer.length, + &handle->response_buffer.dns_response); + } else { + ESP_LOGE(TAG, "DNS response too small"); + handle->response_buffer.dns_response.status_code = ERR_VAL; + } } else { ESP_LOGE(TAG, "HTTP Error: %d", esp_http_client_get_status_code(evt->client)); temp_buff_len = handle->response_buffer.length > ESP_DNS_BUFFER_SIZE ? ESP_DNS_BUFFER_SIZE : handle->response_buffer.length; diff --git a/components/esp_dns/esp_dns_dot.c b/components/esp_dns/esp_dns_dot.c index e7fd97b75..4aaee3894 100644 --- a/components/esp_dns/esp_dns_dot.c +++ b/components/esp_dns/esp_dns_dot.c @@ -173,7 +173,14 @@ err_t dns_resolve_dot(const esp_dns_handle_t handle, const char *name, ip_addr_t dot_buffer, sizeof(dot_buffer), timeout_ms); - if (len > 0) { + if (len >= 2 + sizeof(dns_header_t)) { + size_t expected_len = ((uint8_t)dot_buffer[0] << 8) | (uint8_t)dot_buffer[1]; + if (expected_len > len - 2) { + ESP_LOGE(TAG, "Incomplete DNS response"); + err = ERR_ABRT; + goto cleanup; + } + /* Skip the 2-byte length field that prepends DNS messages as required by RFC 7858 */ handle->response_buffer.buffer = dot_buffer + 2; handle->response_buffer.length = len - 2; @@ -190,7 +197,7 @@ err_t dns_resolve_dot(const esp_dns_handle_t handle, const char *name, ip_addr_t goto cleanup; } } else { - ESP_LOGE(TAG, "Failed to receive response"); + ESP_LOGE(TAG, "Failed to receive response or response too small"); err = ERR_ABRT; goto cleanup; } diff --git a/components/esp_dns/esp_dns_tcp.c b/components/esp_dns/esp_dns_tcp.c index 749dfb0af..8872d64e2 100644 --- a/components/esp_dns/esp_dns_tcp.c +++ b/components/esp_dns/esp_dns_tcp.c @@ -156,7 +156,14 @@ err_t dns_resolve_tcp(const esp_dns_handle_t handle, const char *name, ip_addr_t tcp_buffer, sizeof(tcp_buffer), timeout_ms); - if (len > 0) { + if (len >= 2 + sizeof(dns_header_t)) { + size_t expected_len = ((uint8_t)tcp_buffer[0] << 8) | (uint8_t)tcp_buffer[1]; + if (expected_len > len - 2) { + ESP_LOGE(TAG, "Incomplete DNS response"); + err = ERR_ABRT; + goto cleanup; + } + /* Skip the 2-byte length field that prepends DNS messages as required by RFC 7858 */ handle->response_buffer.buffer = tcp_buffer + 2; handle->response_buffer.length = len - 2; @@ -173,7 +180,7 @@ err_t dns_resolve_tcp(const esp_dns_handle_t handle, const char *name, ip_addr_t goto cleanup; } } else { - ESP_LOGE(TAG, "Failed to receive response"); + ESP_LOGE(TAG, "Failed to receive response or response too small"); err = ERR_ABRT; goto cleanup; } diff --git a/components/esp_dns/esp_dns_utils.c b/components/esp_dns/esp_dns_utils.c index b9726af16..021d5d5dc 100644 --- a/components/esp_dns/esp_dns_utils.c +++ b/components/esp_dns/esp_dns_utils.c @@ -86,17 +86,27 @@ size_t esp_dns_create_query(uint8_t *buffer, size_t buffer_size, const char *hos */ static uint8_t *skip_dns_name(uint8_t *ptr, size_t remaining_bytes) { - uint8_t offset = 0; + size_t offset = 0; /* Loop through each part of the name, handling labels and compression pointers */ - while (ptr[offset] != 0) { + while (1) { if (offset >= remaining_bytes) { return NULL; } + + /* Check for end of name (0x00) */ + if (ptr[offset] == 0) { + offset += 1; + break; + } + /* Check if this part is a compression pointer, indicated by the two high bits set to 1 (0xC0) */ /* RFC 1035, Section 4.1.4: Compression pointers */ if ((ptr[offset] & 0xC0) == 0xC0) { - /* Compression pointer is 2 bytes; move offset by 2 and stop */ + /* Compression pointer is 2 bytes */ + if (remaining_bytes - offset < 2) { + return NULL; + } offset += 2; return ptr + offset; /* End of name processing due to pointer */ } else { @@ -104,12 +114,19 @@ static uint8_t *skip_dns_name(uint8_t *ptr, size_t remaining_bytes) RFC 1035, Section 3.1: Labels - The first byte is the length of this label - Followed by 'length' bytes of label content */ - offset += ptr[offset] + 1; /* Move past this label (1 byte for length + label content) */ + size_t label_len = ptr[offset]; + /* RFC 1035: top two bits of label length must be zero (max 63) */ + if (label_len > 63) { + return NULL; + } + /* 1 byte for length + label_len for content; subtract to avoid overflow */ + if (label_len + 1 > remaining_bytes - offset) { + return NULL; + } + offset += label_len + 1; } } - /* RFC 1035, Section 3.1: End of a name is indicated by a zero-length byte (0x00) */ - offset += 1; /* Move past the terminating zero byte */ return ptr + offset; } @@ -123,13 +140,20 @@ static uint8_t *skip_dns_name(uint8_t *ptr, size_t remaining_bytes) */ void esp_dns_parse_response(uint8_t *buffer, size_t response_size, dns_response_t *dns_response) { - /* Validate input buffer */ + /* Validate input buffer and minimum size */ assert(buffer != NULL); - dns_header_t *header = (dns_header_t *)buffer; - dns_response->status_code = ERR_OK; /* Initialize DNS response code */ + if (response_size < sizeof(dns_header_t)) { + dns_response->status_code = ERR_VAL; + return; + } + + uint8_t *buffer_end = buffer + response_size; + + dns_header_t *header = (dns_header_t *)buffer; + /* Check if there are answers and Transaction id matches */ int answer_count = ntohs(header->ancount); if ((ntohs(header->id) != dns_response->id) || (answer_count == 0)) { @@ -144,25 +168,42 @@ void esp_dns_parse_response(uint8_t *buffer, size_t response_size, dns_response_ uint8_t *ptr = buffer + sizeof(dns_header_t); /* Skip the question name */ - ptr = skip_dns_name(ptr, response_size - (ptr - buffer)); + if (ptr > buffer_end) { + dns_response->status_code = ERR_VAL; + return; + } + ptr = skip_dns_name(ptr, buffer_end - ptr); if (ptr == NULL) { dns_response->status_code = ERR_VAL; return; } /* Skip the question type and class */ + if (ptr + sizeof(dns_question_t) > buffer_end) { + dns_response->status_code = ERR_VAL; + return; + } ptr += sizeof(dns_question_t); /* Parse each answer record */ for (int i = 0; i < dns_response->num_answers; i++) { /* Answer fields */ - ptr = skip_dns_name(ptr, response_size - (ptr - buffer)); + if (ptr > buffer_end) { + dns_response->status_code = ERR_VAL; + return; + } + ptr = skip_dns_name(ptr, buffer_end - ptr); if (ptr == NULL) { dns_response->status_code = ERR_VAL; return; } + if (ptr + SIZEOF_DNS_ANSWER_FIXED > buffer_end) { + dns_response->status_code = ERR_VAL; + return; + } + dns_answer_t *answer = (dns_answer_t *)ptr; uint16_t type = ntohs(answer->type); uint16_t class = ntohs(answer->class); @@ -172,6 +213,11 @@ void esp_dns_parse_response(uint8_t *buffer, size_t response_size, dns_response_ /* Skip fixed parts of answer (type, class, ttl, data_len) */ ptr += SIZEOF_DNS_ANSWER_FIXED; + if (ptr + data_len > buffer_end) { + dns_response->status_code = ERR_VAL; + return; + } + /* Validate RR class and ttl */ if ((class != DNS_RRCLASS_IN) || (ttl > DNS_MAX_TTL)) { dns_response->answers[i].status = ERR_VAL; diff --git a/components/esp_dns/examples/esp_dns_basic/main/esp_dns_example.c b/components/esp_dns/examples/esp_dns_basic/main/esp_dns_example.c index e2440be9c..abfdcbee3 100644 --- a/components/esp_dns/examples/esp_dns_basic/main/esp_dns_example.c +++ b/components/esp_dns/examples/esp_dns_basic/main/esp_dns_example.c @@ -31,6 +31,9 @@ #define TAG "example_esp_dns" +/* DoH runs TLS + esp_http_client on the caller task; 4 KB overflows on IDF 6.x */ +#define ADDR_INFO_TASK_STACK_SIZE (8 * 1024) + extern const char server_root_cert_pem_start[] asm("_binary_cert_google_root_pem_start"); extern const char server_root_cert_pem_end[] asm("_binary_cert_google_root_pem_end"); @@ -137,7 +140,7 @@ static void run_dns_query_task(void) { TaskHandle_t task_handle = NULL; TaskHandle_t parent_handle = xTaskGetCurrentTaskHandle(); - xTaskCreate(addr_info_task, "AddressInfo", 4 * 1024, parent_handle, 5, &task_handle); + xTaskCreate(addr_info_task, "AddressInfo", ADDR_INFO_TASK_STACK_SIZE, parent_handle, 5, &task_handle); /* Wait for task to complete */ if (task_handle != NULL) { @@ -225,8 +228,15 @@ void perform_esp_dns_dot_query(char *val_type) if (strcmp(val_type, "cert") == 0) { dot_config.tls_config.cert_pem = server_root_cert_pem_start; + dot_config.tls_config.crt_bundle_attach = NULL; } else if (strcmp(val_type, "bndl") == 0) { + dot_config.tls_config.cert_pem = NULL; +#if defined(CONFIG_MBEDTLS_CERTIFICATE_BUNDLE) dot_config.tls_config.crt_bundle_attach = esp_crt_bundle_attach; +#else + ESP_LOGW(TAG, "Certificate bundle not enabled; skipping DoT bundle test"); + return; +#endif } /* Initialize DoT DNS module */ @@ -259,14 +269,21 @@ void perform_esp_dns_doh_query(char *val_type) .port = ESP_DNS_DEFAULT_DOH_PORT, .protocol_config.doh_config = { - .url_path = "/dns-query", + .url_path = "dns-query", }, }; if (strcmp(val_type, "cert") == 0) { doh_config.tls_config.cert_pem = server_root_cert_pem_start; + doh_config.tls_config.crt_bundle_attach = NULL; } else if (strcmp(val_type, "bndl") == 0) { + doh_config.tls_config.cert_pem = NULL; +#if defined(CONFIG_MBEDTLS_CERTIFICATE_BUNDLE) doh_config.tls_config.crt_bundle_attach = esp_crt_bundle_attach; +#else + ESP_LOGW(TAG, "Certificate bundle not enabled; skipping DoH bundle test"); + return; +#endif } /* Initialize DoH DNS module */