Merge branch 'bugfix/prov_examples_strlcpy' into 'master'

Wi-Fi Provisioning : Bugfix in copying Wi-Fi SSID and Passphrase

Closes IDF-693

See merge request idf/esp-idf!5180

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;

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;

examples/provisioning/ble_prov/main/app_prov_handlers.c

@@ -98,10 +98,14 @@ static esp_err_t set_config_handler(const wifi_prov_config_set_data_t *req_data,
 
     ESP_LOGI(TAG, "WiFi Credentials Received : \n\tssid %s \n\tpassword %s",
              req_data->ssid, req_data->password);
-    memcpy((char *) wifi_cfg->sta.ssid, req_data->ssid,
-           strnlen(req_data->ssid, sizeof(wifi_cfg->sta.ssid)));
-    memcpy((char *) wifi_cfg->sta.password, req_data->password,
-           strnlen(req_data->password, sizeof(wifi_cfg->sta.password)));
+
+    /* Using strncpy allows the max SSID length to be 32 bytes (as per 802.11 standard).
+     * But this doesn't guarantee that the saved SSID will be null terminated, because
+     * wifi_cfg->sta.ssid is also 32 bytes long (without extra 1 byte for null character).
+     * Although, this is not a matter for concern because esp_wifi library reads the SSID
+     * upto 32 bytes in absence of null termination */
+    strncpy((char *) wifi_cfg->sta.ssid, req_data->ssid, sizeof(wifi_cfg->sta.ssid));
+    strlcpy((char *) wifi_cfg->sta.password, req_data->password, sizeof(wifi_cfg->sta.password));
     return ESP_OK;
 }
 

examples/provisioning/console_prov/main/app_prov_handlers.c

@@ -98,10 +98,14 @@ static esp_err_t set_config_handler(const wifi_prov_config_set_data_t *req_data,
 
     ESP_LOGI(TAG, "WiFi Credentials Received : \n\tssid %s \n\tpassword %s",
              req_data->ssid, req_data->password);
-    memcpy((char *) wifi_cfg->sta.ssid, req_data->ssid,
-           strnlen(req_data->ssid, sizeof(wifi_cfg->sta.ssid)));
-    memcpy((char *) wifi_cfg->sta.password, req_data->password,
-           strnlen(req_data->password, sizeof(wifi_cfg->sta.password)));
+
+    /* Using strncpy allows the max SSID length to be 32 bytes (as per 802.11 standard).
+     * But this doesn't guarantee that the saved SSID will be null terminated, because
+     * wifi_cfg->sta.ssid is also 32 bytes long (without extra 1 byte for null character).
+     * Although, this is not a matter for concern because esp_wifi library reads the SSID
+     * upto 32 bytes in absence of null termination */
+    strncpy((char *) wifi_cfg->sta.ssid, req_data->ssid, sizeof(wifi_cfg->sta.ssid));
+    strlcpy((char *) wifi_cfg->sta.password, req_data->password, sizeof(wifi_cfg->sta.password));
     return ESP_OK;
 }
 

examples/provisioning/custom_config/main/app_prov.c

@@ -303,13 +303,13 @@ static esp_err_t start_wifi_ap(const char *ssid, const char *pass)
     };
 
     strncpy((char *) wifi_config.ap.ssid, ssid, sizeof(wifi_config.ap.ssid));
-    wifi_config.ap.ssid_len = strlen(ssid);
+    wifi_config.ap.ssid_len = strnlen(ssid, sizeof(wifi_config.ap.ssid));
 
     if (strlen(pass) == 0) {
         memset(wifi_config.ap.password, 0, sizeof(wifi_config.ap.password));
         wifi_config.ap.authmode = WIFI_AUTH_OPEN;
     } else {
-        strncpy((char *) wifi_config.ap.password, pass, sizeof(wifi_config.ap.password));
+        strlcpy((char *) wifi_config.ap.password, pass, sizeof(wifi_config.ap.password));
         wifi_config.ap.authmode = WIFI_AUTH_WPA_WPA2_PSK;
     }
 

examples/provisioning/custom_config/main/app_prov_handlers.c

@@ -110,10 +110,14 @@ static esp_err_t set_config_handler(const wifi_prov_config_set_data_t *req_data,
 
     ESP_LOGI(TAG, "WiFi Credentials Received : \n\tssid %s \n\tpassword %s",
              req_data->ssid, req_data->password);
-    memcpy((char *) wifi_cfg->sta.ssid, req_data->ssid,
-           strnlen(req_data->ssid, sizeof(wifi_cfg->sta.ssid)));
-    memcpy((char *) wifi_cfg->sta.password, req_data->password,
-           strnlen(req_data->password, sizeof(wifi_cfg->sta.password)));
+
+    /* Using strncpy allows the max SSID length to be 32 bytes (as per 802.11 standard).
+     * But this doesn't guarantee that the saved SSID will be null terminated, because
+     * wifi_cfg->sta.ssid is also 32 bytes long (without extra 1 byte for null character).
+     * Although, this is not a matter for concern because esp_wifi library reads the SSID
+     * upto 32 bytes in absence of null termination */
+    strncpy((char *) wifi_cfg->sta.ssid, req_data->ssid, sizeof(wifi_cfg->sta.ssid));
+    strlcpy((char *) wifi_cfg->sta.password, req_data->password, sizeof(wifi_cfg->sta.password));
     return ESP_OK;
 }
 

examples/provisioning/softap_prov/main/app_prov.c

@@ -289,13 +289,13 @@ static esp_err_t start_wifi_ap(const char *ssid, const char *pass)
     };
 
     strncpy((char *) wifi_config.ap.ssid, ssid, sizeof(wifi_config.ap.ssid));
-    wifi_config.ap.ssid_len = strlen(ssid);
+    wifi_config.ap.ssid_len = strnlen(ssid, sizeof(wifi_config.ap.ssid));
 
     if (strlen(pass) == 0) {
         memset(wifi_config.ap.password, 0, sizeof(wifi_config.ap.password));
         wifi_config.ap.authmode = WIFI_AUTH_OPEN;
     } else {
-        strncpy((char *) wifi_config.ap.password, pass, sizeof(wifi_config.ap.password));
+        strlcpy((char *) wifi_config.ap.password, pass, sizeof(wifi_config.ap.password));
         wifi_config.ap.authmode = WIFI_AUTH_WPA_WPA2_PSK;
     }
 

examples/provisioning/softap_prov/main/app_prov_handlers.c

@@ -98,10 +98,14 @@ static esp_err_t set_config_handler(const wifi_prov_config_set_data_t *req_data,
 
     ESP_LOGI(TAG, "WiFi Credentials Received : \n\tssid %s \n\tpassword %s",
              req_data->ssid, req_data->password);
-    memcpy((char *) wifi_cfg->sta.ssid, req_data->ssid,
-           strnlen(req_data->ssid, sizeof(wifi_cfg->sta.ssid)));
-    memcpy((char *) wifi_cfg->sta.password, req_data->password,
-           strnlen(req_data->password, sizeof(wifi_cfg->sta.password)));
+
+    /* Using strncpy allows the max SSID length to be 32 bytes (as per 802.11 standard).
+     * But this doesn't guarantee that the saved SSID will be null terminated, because
+     * wifi_cfg->sta.ssid is also 32 bytes long (without extra 1 byte for null character).
+     * Although, this is not a matter for concern because esp_wifi library reads the SSID
+     * upto 32 bytes in absence of null termination */
+    strncpy((char *) wifi_cfg->sta.ssid, req_data->ssid, sizeof(wifi_cfg->sta.ssid));
+    strlcpy((char *) wifi_cfg->sta.password, req_data->password, sizeof(wifi_cfg->sta.password));
     return ESP_OK;
 }