fix(wifi_provisioning): fixes memory leak on OOM

This commit is contained in:
Ashish Sharma
2026-03-26 14:54:19 +08:00
parent 0f294a2d96
commit c692c1ec8b
2 changed files with 53 additions and 23 deletions
+22 -11
View File
@@ -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;
}
+31 -12
View File
@@ -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");