fix(dns): harden DNS response parsing against DoS and OOB reads (BBP#494)

Fix ESP-IDF-ESP_DNS-DOS-001 by making esp_dns response parsing
strictly bounds-safe. A malicious DNS response could previously cause
an infinite loop or read past the end of the receive buffer.

- skip_dns_name(): use size_t offset to prevent wraparound, and check
  bounds before reading labels and compression pointers
- esp_dns_parse_response(): reject responses shorter than a DNS header,
  track buffer_end, guard pointer arithmetic, and validate answer
  record headers and RR data before memcpy
- TCP/DoT/DoH callers: require a minimum response size and verify the
  RFC 7858 length prefix before parsing
This commit is contained in:
Abhik Roy
2026-06-11 22:29:13 +10:00
parent 6cbef7f8fc
commit 39283bb1bb
5 changed files with 103 additions and 21 deletions
+9 -4
View File
@@ -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;
+9 -2
View File
@@ -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;
}
+9 -2
View File
@@ -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;
}
+57 -11
View File
@@ -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;
@@ -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 */