From 0f294a2d96247983f9b0a3f4a708b408a355c867 Mon Sep 17 00:00:00 2001 From: Ashish Sharma Date: Thu, 26 Mar 2026 11:11:08 +0800 Subject: [PATCH 1/2] fix(wifi_provisioning): fixes potential null dereference on malformed packet --- .../wifi_provisioning/src/wifi_config.c | 10 +++++- components/wifi_provisioning/src/wifi_scan.c | 35 ++++++++++++------- tools/ci/check_copyright_ignore.txt | 1 - 3 files changed, 31 insertions(+), 15 deletions(-) diff --git a/components/wifi_provisioning/src/wifi_config.c b/components/wifi_provisioning/src/wifi_config.c index b97ef2a552..23eaa4eeb2 100644 --- a/components/wifi_provisioning/src/wifi_config.c +++ b/components/wifi_provisioning/src/wifi_config.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2018-2024 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2018-2026 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -155,6 +155,14 @@ static esp_err_t cmd_set_config_handler(WiFiConfigPayload *req, } resp_set_config__init(resp_payload); + if (req->payload_case != WI_FI_CONFIG_PAYLOAD__PAYLOAD_CMD_SET_CONFIG || !req->cmd_set_config) { + ESP_LOGE(TAG, "Invalid set config command"); + resp_payload->status = STATUS__InvalidArgument; + resp->payload_case = WI_FI_CONFIG_PAYLOAD__PAYLOAD_RESP_SET_CONFIG; + resp->resp_set_config = resp_payload; + return ESP_OK; + } + wifi_prov_config_set_data_t req_data; memset(&req_data, 0, sizeof(req_data)); diff --git a/components/wifi_provisioning/src/wifi_scan.c b/components/wifi_provisioning/src/wifi_scan.c index bfc0596f52..356bb8dd6f 100644 --- a/components/wifi_provisioning/src/wifi_scan.c +++ b/components/wifi_provisioning/src/wifi_scan.c @@ -1,16 +1,8 @@ -// Copyright 2019 Espressif Systems (Shanghai) PTE LTD -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. +/* + * SPDX-FileCopyrightText: 2019-2026 Espressif Systems (Shanghai) CO LTD + * + * SPDX-License-Identifier: Apache-2.0 + */ #include #include @@ -73,6 +65,15 @@ static esp_err_t cmd_scan_start_handler(WiFiScanPayload *req, } resp_scan_start__init(resp_payload); + + if (req->payload_case != WI_FI_SCAN_PAYLOAD__PAYLOAD_CMD_SCAN_START || !req->cmd_scan_start) { + ESP_LOGE(TAG, "Invalid scan start command"); + resp->status = STATUS__InvalidArgument; + resp->payload_case = WI_FI_SCAN_PAYLOAD__PAYLOAD_RESP_SCAN_START; + resp->resp_scan_start = resp_payload; + return ESP_OK; + } + resp->status = (h->scan_start(req->cmd_scan_start->blocking, req->cmd_scan_start->passive, req->cmd_scan_start->group_channels, @@ -131,6 +132,14 @@ static esp_err_t cmd_scan_result_handler(WiFiScanPayload *req, } resp_scan_result__init(resp_payload); + if (req->payload_case != WI_FI_SCAN_PAYLOAD__PAYLOAD_CMD_SCAN_RESULT || !req->cmd_scan_result) { + ESP_LOGE(TAG, "Invalid scan result command"); + resp->status = STATUS__InvalidArgument; + resp->payload_case = WI_FI_SCAN_PAYLOAD__PAYLOAD_RESP_SCAN_RESULT; + resp->resp_scan_result = resp_payload; + return ESP_OK; + } + resp->status = STATUS__Success; resp->payload_case = WI_FI_SCAN_PAYLOAD__PAYLOAD_RESP_SCAN_RESULT; resp->resp_scan_result = resp_payload; diff --git a/tools/ci/check_copyright_ignore.txt b/tools/ci/check_copyright_ignore.txt index 8d750f572a..a77699a92b 100644 --- a/tools/ci/check_copyright_ignore.txt +++ b/tools/ci/check_copyright_ignore.txt @@ -531,7 +531,6 @@ components/wifi_provisioning/python/wifi_config_pb2.py components/wifi_provisioning/python/wifi_constants_pb2.py components/wifi_provisioning/python/wifi_scan_pb2.py components/wifi_provisioning/src/scheme_console.c -components/wifi_provisioning/src/wifi_scan.c components/wpa_supplicant/esp_supplicant/src/esp_wpa_err.h components/wpa_supplicant/include/utils/wpa_debug.h components/wpa_supplicant/include/utils/wpabuf.h From c692c1ec8b73e2356f5ce2f782c9681be5eeb542 Mon Sep 17 00:00:00 2001 From: Ashish Sharma Date: Thu, 26 Mar 2026 14:54:19 +0800 Subject: [PATCH 2/2] fix(wifi_provisioning): fixes memory leak on OOM --- .../wifi_provisioning/src/wifi_config.c | 33 +++++++++----- components/wifi_provisioning/src/wifi_scan.c | 43 +++++++++++++------ 2 files changed, 53 insertions(+), 23 deletions(-) diff --git a/components/wifi_provisioning/src/wifi_config.c b/components/wifi_provisioning/src/wifi_config.c index 23eaa4eeb2..077f9fe686 100644 --- a/components/wifi_provisioning/src/wifi_config.c +++ b/components/wifi_provisioning/src/wifi_config.c @@ -83,6 +83,7 @@ static esp_err_t cmd_get_status_handler(WiFiConfigPayload *req, connected->ip4_addr = strdup(resp_data.conn_info.ip_addr); if (connected->ip4_addr == NULL) { + free(connected); free(resp_payload); return ESP_ERR_NO_MEM; } @@ -92,6 +93,7 @@ static esp_err_t cmd_get_status_handler(WiFiConfigPayload *req, sizeof(resp_data.conn_info.bssid)); if (connected->bssid.data == NULL) { free(connected->ip4_addr); + free(connected); free(resp_payload); return ESP_ERR_NO_MEM; } @@ -101,6 +103,7 @@ static esp_err_t cmd_get_status_handler(WiFiConfigPayload *req, if (connected->ssid.data == NULL) { free(connected->bssid.data); free(connected->ip4_addr); + free(connected); free(resp_payload); return ESP_ERR_NO_MEM; } @@ -157,10 +160,8 @@ static esp_err_t cmd_set_config_handler(WiFiConfigPayload *req, if (req->payload_case != WI_FI_CONFIG_PAYLOAD__PAYLOAD_CMD_SET_CONFIG || !req->cmd_set_config) { ESP_LOGE(TAG, "Invalid set config command"); - resp_payload->status = STATUS__InvalidArgument; - resp->payload_case = WI_FI_CONFIG_PAYLOAD__PAYLOAD_RESP_SET_CONFIG; resp->resp_set_config = resp_payload; - return ESP_OK; + return ESP_ERR_INVALID_ARG; } wifi_prov_config_set_data_t req_data; @@ -250,6 +251,9 @@ static void wifi_prov_config_command_cleanup(WiFiConfigPayload *resp, void *priv switch (resp->msg) { case WI_FI_CONFIG_MSG_TYPE__TypeRespGetStatus: { + if (!resp->resp_get_status) { + break; + } switch (resp->resp_get_status->sta_state) { case WIFI_STATION_STATE__Connecting: break; @@ -328,28 +332,35 @@ esp_err_t wifi_prov_config_data_handler(uint32_t session_id, const uint8_t *inbu } wi_fi_config_payload__init(&resp); + /* Validate req->msg before arithmetic to avoid signed overflow on attacker-controlled + * wire values. For unknown commands the dispatcher returns ESP_FAIL without calling + * any handler, so nothing is allocated and resp.msg = 0 is safe for cleanup. */ + if (lookup_cmd_handler(req->msg) >= 0) { + resp.msg = req->msg + 1; + } ret = wifi_prov_config_command_dispatcher(req, &resp, priv_data); if (ret != ESP_OK) { ESP_LOGE(TAG, "Proto command dispatcher error %d", ret); - return ESP_FAIL; + ret = ESP_FAIL; + goto exit; } - resp.msg = req->msg + 1; /* Response is request + 1 */ - wi_fi_config_payload__free_unpacked(req, NULL); - *outlen = wi_fi_config_payload__get_packed_size(&resp); if (*outlen <= 0) { ESP_LOGE(TAG, "Invalid encoding for response"); - return ESP_FAIL; + ret = ESP_FAIL; + goto exit; } *outbuf = (uint8_t *) malloc(*outlen); if (!*outbuf) { ESP_LOGE(TAG, "System out of memory"); - return ESP_ERR_NO_MEM; + ret = ESP_ERR_NO_MEM; + goto exit; } wi_fi_config_payload__pack(&resp, *outbuf); +exit: + wi_fi_config_payload__free_unpacked(req, NULL); wifi_prov_config_command_cleanup(&resp, priv_data); - - return ESP_OK; + return ret; } diff --git a/components/wifi_provisioning/src/wifi_scan.c b/components/wifi_provisioning/src/wifi_scan.c index 356bb8dd6f..fdfcb6e255 100644 --- a/components/wifi_provisioning/src/wifi_scan.c +++ b/components/wifi_provisioning/src/wifi_scan.c @@ -68,10 +68,8 @@ static esp_err_t cmd_scan_start_handler(WiFiScanPayload *req, if (req->payload_case != WI_FI_SCAN_PAYLOAD__PAYLOAD_CMD_SCAN_START || !req->cmd_scan_start) { ESP_LOGE(TAG, "Invalid scan start command"); - resp->status = STATUS__InvalidArgument; - resp->payload_case = WI_FI_SCAN_PAYLOAD__PAYLOAD_RESP_SCAN_START; resp->resp_scan_start = resp_payload; - return ESP_OK; + return ESP_ERR_INVALID_ARG; } resp->status = (h->scan_start(req->cmd_scan_start->blocking, @@ -134,10 +132,16 @@ static esp_err_t cmd_scan_result_handler(WiFiScanPayload *req, if (req->payload_case != WI_FI_SCAN_PAYLOAD__PAYLOAD_CMD_SCAN_RESULT || !req->cmd_scan_result) { ESP_LOGE(TAG, "Invalid scan result command"); - resp->status = STATUS__InvalidArgument; - resp->payload_case = WI_FI_SCAN_PAYLOAD__PAYLOAD_RESP_SCAN_RESULT; resp->resp_scan_result = resp_payload; - return ESP_OK; + return ESP_ERR_INVALID_ARG; + } + + if (req->cmd_scan_result->start_index >= CONFIG_WIFI_PROV_SCAN_MAX_ENTRIES || + req->cmd_scan_result->count > + CONFIG_WIFI_PROV_SCAN_MAX_ENTRIES - req->cmd_scan_result->start_index) { + ESP_LOGE(TAG, "Scan result count/start_index out of bounds"); + resp->resp_scan_result = resp_payload; + return ESP_ERR_INVALID_ARG; } resp->status = STATUS__Success; @@ -158,10 +162,13 @@ static esp_err_t cmd_scan_result_handler(WiFiScanPayload *req, /* If req->cmd_scan_result->count is 0, the below loop will * be skipped. */ - for (uint16_t i = 0; i < req->cmd_scan_result->count; i++) { - err = h->scan_result(i + req->cmd_scan_result->start_index, - &scan_result, &h->ctx); + for (uint32_t i = 0; i < req->cmd_scan_result->count; i++) { + /* start_index and count are validated above to sum to at most + * CONFIG_WIFI_PROV_SCAN_MAX_ENTRIES (max 255), so this cast is safe. */ + uint16_t result_index = (uint16_t)(i + req->cmd_scan_result->start_index); + err = h->scan_result(result_index, &scan_result, &h->ctx); if (err != ESP_OK) { + resp_payload->n_entries = i; resp->status = STATUS__InternalError; break; } @@ -169,6 +176,8 @@ static esp_err_t cmd_scan_result_handler(WiFiScanPayload *req, results[i] = (WiFiScanResult *) malloc(sizeof(WiFiScanResult)); if (!results[i]) { ESP_LOGE(TAG, "Failed to allocate memory for result entry"); + resp_payload->n_entries = i; + resp->status = STATUS__InternalError; return ESP_ERR_NO_MEM; } wi_fi_scan_result__init(results[i]); @@ -177,6 +186,9 @@ static esp_err_t cmd_scan_result_handler(WiFiScanPayload *req, results[i]->ssid.data = (uint8_t *) strndup(scan_result.ssid, 32); if (!results[i]->ssid.data) { ESP_LOGE(TAG, "Failed to allocate memory for scan result entry SSID"); + results[i]->ssid.len = 0; + resp_payload->n_entries = i + 1; + resp->status = STATUS__InternalError; return ESP_ERR_NO_MEM; } @@ -188,6 +200,9 @@ static esp_err_t cmd_scan_result_handler(WiFiScanPayload *req, results[i]->bssid.data = malloc(results[i]->bssid.len); if (!results[i]->bssid.data) { ESP_LOGE(TAG, "Failed to allocate memory for scan result entry BSSID"); + results[i]->bssid.len = 0; + resp_payload->n_entries = i + 1; + resp->status = STATUS__InternalError; return ESP_ERR_NO_MEM; } memcpy(results[i]->bssid.data, scan_result.bssid, results[i]->bssid.len); @@ -224,7 +239,7 @@ static void wifi_prov_scan_cmd_cleanup(WiFiScanPayload *resp, void *priv_data) { if (!resp->resp_scan_result) return; if (resp->resp_scan_result->entries) { - for (uint16_t i = 0; i < resp->resp_scan_result->n_entries; i++) { + for (uint32_t i = 0; i < resp->resp_scan_result->n_entries; i++) { if (!resp->resp_scan_result->entries[i]) continue; free(resp->resp_scan_result->entries[i]->ssid.data); free(resp->resp_scan_result->entries[i]->bssid.data); @@ -278,14 +293,18 @@ esp_err_t wifi_prov_scan_handler(uint32_t session_id, const uint8_t *inbuf, ssiz } wi_fi_scan_payload__init(&resp); + /* Validate req->msg before arithmetic to avoid signed overflow on attacker-controlled + * wire values. For unknown commands the dispatcher returns ESP_FAIL without calling + * any handler, so nothing is allocated and resp.msg = 0 is safe for cleanup. */ + if (lookup_cmd_handler(req->msg) >= 0) { + resp.msg = req->msg + 1; + } ret = wifi_prov_scan_cmd_dispatcher(req, &resp, priv_data); if (ret != ESP_OK) { ESP_LOGE(TAG, "Command dispatcher error %d", ret); ret = ESP_FAIL; goto exit; } - - resp.msg = req->msg + 1; /* Response is request + 1 */ *outlen = wi_fi_scan_payload__get_packed_size(&resp); if (*outlen <= 0) { ESP_LOGE(TAG, "Invalid encoding for response");