mirror of
https://github.com/espressif/esp-protocols.git
synced 2026-07-07 17:10:52 +02:00
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:
@@ -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;
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
|
||||
@@ -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 */
|
||||
|
||||
Reference in New Issue
Block a user