From a75be3413e6251e17ce5efd7581c04548d2ac2c1 Mon Sep 17 00:00:00 2001 From: Anurag Kar Date: Mon, 10 Jun 2019 17:49:30 +0530 Subject: [PATCH] Wi-Fi Provisioning : Bugfix in copying SSID and Passphrase These changes guarantee that the SSID and Passphrase received via protocomm are NULL terminated and size limited to their standard lengths. List of changes: * Corrected length of passphrase field in wifi_prov_config_set_data_t structure * Performing length checks on SSID, passphrase and bssid, when populating wifi_prov_config_set_data_t structure with received credentials --- .../include/wifi_provisioning/wifi_config.h | 2 +- .../wifi_provisioning/src/wifi_config.c | 41 ++++++++++++++----- 2 files changed, 32 insertions(+), 11 deletions(-) diff --git a/components/wifi_provisioning/include/wifi_provisioning/wifi_config.h b/components/wifi_provisioning/include/wifi_provisioning/wifi_config.h index 4058540adb..2fa64448d4 100644 --- a/components/wifi_provisioning/include/wifi_provisioning/wifi_config.h +++ b/components/wifi_provisioning/include/wifi_provisioning/wifi_config.h @@ -76,7 +76,7 @@ typedef struct { */ typedef struct { char ssid[33]; /*!< SSID of the AP to which the slave is to be connected */ - char password[65]; /*!< Password of the AP */ + char password[64]; /*!< Password of the AP */ char bssid[6]; /*!< BSSID of the AP */ uint8_t channel; /*!< Channel of the AP */ } wifi_prov_config_set_data_t; diff --git a/components/wifi_provisioning/src/wifi_config.c b/components/wifi_provisioning/src/wifi_config.c index 09ccc37d53..93e3e85643 100644 --- a/components/wifi_provisioning/src/wifi_config.c +++ b/components/wifi_provisioning/src/wifi_config.c @@ -151,15 +151,36 @@ static esp_err_t cmd_set_config_handler(WiFiConfigPayload *req, wifi_prov_config_set_data_t req_data; memset(&req_data, 0, sizeof(req_data)); - memcpy(req_data.ssid, req->cmd_set_config->ssid.data, - req->cmd_set_config->ssid.len); - memcpy(req_data.password, req->cmd_set_config->passphrase.data, - req->cmd_set_config->passphrase.len); - memcpy(req_data.bssid, req->cmd_set_config->bssid.data, - req->cmd_set_config->bssid.len); - req_data.channel = req->cmd_set_config->channel; - if (h->set_config_handler(&req_data, &h->ctx) == ESP_OK) { - resp_payload->status = STATUS__Success; + + /* Check arguments provided in protobuf packet: + * - SSID / Passphrase string length must be within the standard limits + * - BSSID must either be NULL or have length equal to that imposed by the standard + * If any of these conditions are not satisfied, don't invoke the handler and + * send error status without closing connection */ + resp_payload->status = STATUS__InvalidArgument; + if (req->cmd_set_config->bssid.len != 0 && + req->cmd_set_config->bssid.len != sizeof(req_data.bssid)) { + ESP_LOGD(TAG, "Received invalid BSSID"); + } else if (req->cmd_set_config->ssid.len >= sizeof(req_data.ssid)) { + ESP_LOGD(TAG, "Received invalid SSID"); + } else if (req->cmd_set_config->passphrase.len >= sizeof(req_data.password)) { + ESP_LOGD(TAG, "Received invalid Passphrase"); + } else { + /* The received SSID and Passphrase are not NULL terminated so + * we memcpy over zeroed out arrays. Above length checks ensure + * that there is atleast 1 extra byte for null termination */ + memcpy(req_data.ssid, req->cmd_set_config->ssid.data, + req->cmd_set_config->ssid.len); + memcpy(req_data.password, req->cmd_set_config->passphrase.data, + req->cmd_set_config->passphrase.len); + memcpy(req_data.bssid, req->cmd_set_config->bssid.data, + req->cmd_set_config->bssid.len); + req_data.channel = req->cmd_set_config->channel; + if (h->set_config_handler(&req_data, &h->ctx) == ESP_OK) { + resp_payload->status = STATUS__Success; + } else { + resp_payload->status = STATUS__InternalError; + } } resp->payload_case = WI_FI_CONFIG_PAYLOAD__PAYLOAD_RESP_SET_CONFIG; @@ -188,7 +209,7 @@ static esp_err_t cmd_apply_config_handler(WiFiConfigPayload *req, if (h->apply_config_handler(&h->ctx) == ESP_OK) { resp_payload->status = STATUS__Success; } else { - resp_payload->status = STATUS__InvalidArgument; + resp_payload->status = STATUS__InternalError; } resp->payload_case = WI_FI_CONFIG_PAYLOAD__PAYLOAD_RESP_APPLY_CONFIG;