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");