Skip to content

Commit 55b82a2

Browse files
Ashish285Ashish285
committed
fix(network_provisioning): adds more checks for externally received messages
1 parent 78a1615 commit 55b82a2

2 files changed

Lines changed: 107 additions & 19 deletions

File tree

‎network_provisioning/src/network_config.c‎

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -246,6 +246,13 @@ static esp_err_t cmd_set_config_handler(NetworkConfigPayload *req,
246246
return ESP_ERR_NO_MEM;
247247
}
248248
resp_set_wifi_config__init(resp_payload);
249+
250+
if (req->payload_case != NETWORK_CONFIG_PAYLOAD__PAYLOAD_CMD_SET_WIFI_CONFIG || !req->cmd_set_wifi_config) {
251+
ESP_LOGE(TAG, "Invalid set WiFi config command");
252+
free(resp_payload);
253+
return ESP_ERR_INVALID_ARG;
254+
}
255+
249256
#ifdef CONFIG_NETWORK_PROV_NETWORK_TYPE_WIFI
250257
network_prov_config_set_wifi_data_t req_data;
251258
memset(&req_data, 0, sizeof(req_data));
@@ -292,20 +299,28 @@ static esp_err_t cmd_set_config_handler(NetworkConfigPayload *req,
292299
return ESP_ERR_NO_MEM;
293300
}
294301
resp_set_thread_config__init(resp_payload);
302+
303+
if (req->payload_case != NETWORK_CONFIG_PAYLOAD__PAYLOAD_CMD_SET_THREAD_CONFIG || !req->cmd_set_thread_config) {
304+
ESP_LOGE(TAG, "Invalid set Thread config command");
305+
free(resp_payload);
306+
return ESP_ERR_INVALID_ARG;
307+
}
308+
295309
#ifdef CONFIG_NETWORK_PROV_NETWORK_TYPE_THREAD
296310
network_prov_config_set_thread_data_t req_data;
297311
memset(&req_data, 0, sizeof(req_data));
298312
resp_payload->status = STATUS__InvalidArgument;
299313
if (req->cmd_set_thread_config->dataset.len > sizeof(req_data.dataset)) {
300314
ESP_LOGD(TAG, "Received invalid dataset");
301-
}
302-
memcpy(req_data.dataset, req->cmd_set_thread_config->dataset.data,
303-
req->cmd_set_thread_config->dataset.len);
304-
req_data.length = req->cmd_set_thread_config->dataset.len;
305-
if (h->thread_set_config_handler(&req_data, &h->ctx) == ESP_OK) {
306-
resp_payload->status = STATUS__Success;
307315
} else {
308-
resp_payload->status = STATUS__InternalError;
316+
memcpy(req_data.dataset, req->cmd_set_thread_config->dataset.data,
317+
req->cmd_set_thread_config->dataset.len);
318+
req_data.length = req->cmd_set_thread_config->dataset.len;
319+
if (h->thread_set_config_handler(&req_data, &h->ctx) == ESP_OK) {
320+
resp_payload->status = STATUS__Success;
321+
} else {
322+
resp_payload->status = STATUS__InternalError;
323+
}
309324
}
310325
#else // CONFIG_NETWORK_PROV_NETWORK_TYPE_THREAD
311326
resp_payload->status = STATUS__InvalidArgument;

‎network_provisioning/src/network_scan.c‎

Lines changed: 85 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,13 @@ static esp_err_t cmd_scan_start_handler(NetworkScanPayload *req,
7676
return ESP_ERR_NO_MEM;
7777
}
7878
resp_scan_wifi_start__init(resp_payload);
79+
80+
if (req->payload_case != NETWORK_SCAN_PAYLOAD__PAYLOAD_CMD_SCAN_WIFI_START || !req->cmd_scan_wifi_start) {
81+
ESP_LOGE(TAG, "Invalid WiFi scan start command");
82+
resp->resp_scan_wifi_start = resp_payload;
83+
return ESP_ERR_INVALID_ARG;
84+
}
85+
7986
#ifdef CONFIG_NETWORK_PROV_NETWORK_TYPE_WIFI
8087
if (h->wifi_scan_start) {
8188
resp->status = (h->wifi_scan_start(req->cmd_scan_wifi_start->blocking,
@@ -98,6 +105,13 @@ static esp_err_t cmd_scan_start_handler(NetworkScanPayload *req,
98105
return ESP_ERR_NO_MEM;
99106
}
100107
resp_scan_thread_start__init(resp_payload);
108+
109+
if (req->payload_case != NETWORK_SCAN_PAYLOAD__PAYLOAD_CMD_SCAN_THREAD_START || !req->cmd_scan_thread_start) {
110+
ESP_LOGE(TAG, "Invalid Thread scan start command");
111+
resp->resp_scan_thread_start = resp_payload;
112+
return ESP_ERR_INVALID_ARG;
113+
}
114+
101115
#ifdef CONFIG_NETWORK_PROV_NETWORK_TYPE_THREAD
102116
if (h->thread_scan_start) {
103117
resp->status = (h->thread_scan_start(req->cmd_scan_thread_start->blocking,
@@ -188,6 +202,21 @@ static esp_err_t cmd_scan_result_handler(NetworkScanPayload *req,
188202
return ESP_ERR_NO_MEM;
189203
}
190204
resp_scan_wifi_result__init(resp_payload);
205+
206+
if (req->payload_case != NETWORK_SCAN_PAYLOAD__PAYLOAD_CMD_SCAN_WIFI_RESULT || !req->cmd_scan_wifi_result) {
207+
ESP_LOGE(TAG, "Invalid WiFi scan result command");
208+
resp->resp_scan_wifi_result = resp_payload;
209+
return ESP_ERR_INVALID_ARG;
210+
}
211+
212+
if (req->cmd_scan_wifi_result->start_index >= CONFIG_NETWORK_PROV_SCAN_MAX_ENTRIES ||
213+
req->cmd_scan_wifi_result->count >
214+
CONFIG_NETWORK_PROV_SCAN_MAX_ENTRIES - req->cmd_scan_wifi_result->start_index) {
215+
ESP_LOGE(TAG, "WiFi scan result count/start_index out of bounds");
216+
resp->resp_scan_wifi_result = resp_payload;
217+
return ESP_ERR_INVALID_ARG;
218+
}
219+
191220
resp->status = STATUS__Success;
192221
resp->payload_case = NETWORK_SCAN_PAYLOAD__PAYLOAD_RESP_SCAN_WIFI_RESULT;
193222
resp->resp_scan_wifi_result = resp_payload;
@@ -210,21 +239,27 @@ static esp_err_t cmd_scan_result_handler(NetworkScanPayload *req,
210239
/* If req->cmd_scan_wifi_result->count is 0, the below loop will
211240
* be skipped.
212241
*/
213-
for (uint16_t i = 0; i < req->cmd_scan_wifi_result->count; i++) {
242+
for (uint32_t i = 0; i < req->cmd_scan_wifi_result->count; i++) {
214243
if (!h->wifi_scan_result) {
244+
resp_payload->n_entries = i;
215245
resp->status = STATUS__InternalError;
216246
break;
217247
}
218-
err = h->wifi_scan_result(i + req->cmd_scan_wifi_result->start_index,
219-
&scan_result, &h->ctx);
248+
/* start_index and count are validated above to sum to at most
249+
* CONFIG_NETWORK_PROV_SCAN_MAX_ENTRIES (max 255), so this cast is safe. */
250+
uint16_t result_index = (uint16_t)(i + req->cmd_scan_wifi_result->start_index);
251+
err = h->wifi_scan_result(result_index, &scan_result, &h->ctx);
220252
if (err != ESP_OK) {
253+
resp_payload->n_entries = i;
221254
resp->status = STATUS__InternalError;
222255
break;
223256
}
224257

225258
results[i] = (WiFiScanResult *) malloc(sizeof(WiFiScanResult));
226259
if (!results[i]) {
227260
ESP_LOGE(TAG, "Failed to allocate memory for result entry");
261+
resp_payload->n_entries = i;
262+
resp->status = STATUS__InternalError;
228263
return ESP_ERR_NO_MEM;
229264
}
230265
wi_fi_scan_result__init(results[i]);
@@ -233,6 +268,9 @@ static esp_err_t cmd_scan_result_handler(NetworkScanPayload *req,
233268
results[i]->ssid.data = (uint8_t *) strndup(scan_result.ssid, 32);
234269
if (!results[i]->ssid.data) {
235270
ESP_LOGE(TAG, "Failed to allocate memory for scan result entry SSID");
271+
results[i]->ssid.len = 0;
272+
resp_payload->n_entries = i + 1;
273+
resp->status = STATUS__InternalError;
236274
return ESP_ERR_NO_MEM;
237275
}
238276

@@ -244,6 +282,9 @@ static esp_err_t cmd_scan_result_handler(NetworkScanPayload *req,
244282
results[i]->bssid.data = malloc(results[i]->bssid.len);
245283
if (!results[i]->bssid.data) {
246284
ESP_LOGE(TAG, "Failed to allocate memory for scan result entry BSSID");
285+
results[i]->bssid.len = 0;
286+
resp_payload->n_entries = i + 1;
287+
resp->status = STATUS__InternalError;
247288
return ESP_ERR_NO_MEM;
248289
}
249290
memcpy(results[i]->bssid.data, scan_result.bssid, results[i]->bssid.len);
@@ -258,6 +299,21 @@ static esp_err_t cmd_scan_result_handler(NetworkScanPayload *req,
258299
return ESP_ERR_NO_MEM;
259300
}
260301
resp_scan_thread_result__init(resp_payload);
302+
303+
if (req->payload_case != NETWORK_SCAN_PAYLOAD__PAYLOAD_CMD_SCAN_THREAD_RESULT || !req->cmd_scan_thread_result) {
304+
ESP_LOGE(TAG, "Invalid Thread scan result command");
305+
resp->resp_scan_thread_result = resp_payload;
306+
return ESP_ERR_INVALID_ARG;
307+
}
308+
309+
if (req->cmd_scan_thread_result->start_index >= CONFIG_NETWORK_PROV_SCAN_MAX_ENTRIES ||
310+
req->cmd_scan_thread_result->count >
311+
CONFIG_NETWORK_PROV_SCAN_MAX_ENTRIES - req->cmd_scan_thread_result->start_index) {
312+
ESP_LOGE(TAG, "Thread scan result count/start_index out of bounds");
313+
resp->resp_scan_thread_result = resp_payload;
314+
return ESP_ERR_INVALID_ARG;
315+
}
316+
261317
resp->status = STATUS__Success;
262318
resp->payload_case = NETWORK_SCAN_PAYLOAD__PAYLOAD_RESP_SCAN_THREAD_RESULT;
263319
resp->resp_scan_thread_result = resp_payload;
@@ -281,21 +337,27 @@ static esp_err_t cmd_scan_result_handler(NetworkScanPayload *req,
281337
/* If req->cmd_scan_result->count is 0, the below loop will
282338
* be skipped.
283339
*/
284-
for (uint16_t i = 0; i < req->cmd_scan_thread_result->count; i++) {
340+
for (uint32_t i = 0; i < req->cmd_scan_thread_result->count; i++) {
285341
if (!h->thread_scan_result) {
342+
resp_payload->n_entries = i;
286343
resp->status = STATUS__InternalError;
287344
break;
288345
}
289-
err = h->thread_scan_result(i + req->cmd_scan_thread_result->start_index,
290-
&scan_result, &h->ctx);
346+
/* start_index and count are validated above to sum to at most
347+
* CONFIG_NETWORK_PROV_SCAN_MAX_ENTRIES (max 255), so this cast is safe. */
348+
uint16_t result_index = (uint16_t)(i + req->cmd_scan_thread_result->start_index);
349+
err = h->thread_scan_result(result_index, &scan_result, &h->ctx);
291350
if (err != ESP_OK) {
351+
resp_payload->n_entries = i;
292352
resp->status = STATUS__InternalError;
293353
break;
294354
}
295355

296356
results[i] = (ThreadScanResult *) malloc(sizeof(ThreadScanResult));
297357
if (!results[i]) {
298358
ESP_LOGE(TAG, "Failed to allocate memory for result entry");
359+
resp_payload->n_entries = i;
360+
resp->status = STATUS__InternalError;
299361
return ESP_ERR_NO_MEM;
300362
}
301363
thread_scan_result__init(results[i]);
@@ -308,6 +370,9 @@ static esp_err_t cmd_scan_result_handler(NetworkScanPayload *req,
308370
results[i]->ext_addr.data = (uint8_t *)malloc(results[i]->ext_addr.len);
309371
if (!results[i]->ext_addr.data) {
310372
ESP_LOGE(TAG, "Failed to allocate memory for scan result entry extended address");
373+
results[i]->ext_addr.len = 0;
374+
resp_payload->n_entries = i + 1;
375+
resp->status = STATUS__InternalError;
311376
return ESP_ERR_NO_MEM;
312377
}
313378
memcpy(results[i]->ext_addr.data, scan_result.ext_addr, results[i]->ext_addr.len);
@@ -316,13 +381,19 @@ static esp_err_t cmd_scan_result_handler(NetworkScanPayload *req,
316381
results[i]->ext_pan_id.data = (uint8_t *)malloc(results[i]->ext_pan_id.len);
317382
if (!results[i]->ext_pan_id.data) {
318383
ESP_LOGE(TAG, "Failed to allocate memory for scan result entry extended PAN ID");
384+
results[i]->ext_pan_id.len = 0;
385+
resp_payload->n_entries = i + 1;
386+
resp->status = STATUS__InternalError;
319387
return ESP_ERR_NO_MEM;
320388
}
321389
memcpy(results[i]->ext_pan_id.data, scan_result.ext_pan_id, results[i]->ext_pan_id.len);
322390

323391
results[i]->network_name = (char *)malloc(sizeof(scan_result.network_name));
324392
if (!results[i]->network_name) {
325-
ESP_LOGE(TAG, "Failed to allocate memory for scan result entry networkname");
393+
ESP_LOGE(TAG, "Failed to allocate memory for scan result entry network name");
394+
resp_payload->n_entries = i + 1;
395+
resp->status = STATUS__InternalError;
396+
return ESP_ERR_NO_MEM;
326397
}
327398
memcpy(results[i]->network_name, scan_result.network_name, sizeof(scan_result.network_name));
328399
}
@@ -372,12 +443,13 @@ static void network_prov_scan_cmd_cleanup(NetworkScanPayload *resp, void *priv_d
372443
}
373444
#ifdef CONFIG_NETWORK_PROV_NETWORK_TYPE_WIFI
374445
if (resp->resp_scan_wifi_result->entries) {
375-
for (uint16_t i = 0; i < resp->resp_scan_wifi_result->n_entries; i++) {
446+
for (uint32_t i = 0; i < resp->resp_scan_wifi_result->n_entries; i++) {
376447
if (!resp->resp_scan_wifi_result->entries[i]) {
377448
continue;
378449
}
379450
free(resp->resp_scan_wifi_result->entries[i]->ssid.data);
380451
free(resp->resp_scan_wifi_result->entries[i]->bssid.data);
452+
free(resp->resp_scan_wifi_result->entries[i]);
381453
}
382454
free(resp->resp_scan_wifi_result->entries);
383455
}
@@ -391,13 +463,15 @@ static void network_prov_scan_cmd_cleanup(NetworkScanPayload *resp, void *priv_d
391463
}
392464
#ifdef CONFIG_NETWORK_PROV_NETWORK_TYPE_THREAD
393465
if (resp->resp_scan_thread_result->entries) {
394-
for (uint16_t i = 0; i < resp->resp_scan_thread_result->n_entries; i++) {
466+
for (uint32_t i = 0; i < resp->resp_scan_thread_result->n_entries; i++) {
395467
if (!resp->resp_scan_thread_result->entries[i]) {
396468
continue;
397469
}
398470
free(resp->resp_scan_thread_result->entries[i]->ext_addr.data);
399471
free(resp->resp_scan_thread_result->entries[i]->ext_pan_id.data);
400-
free(resp->resp_scan_thread_result->entries[i]->network_name);
472+
if (resp->resp_scan_thread_result->entries[i]->network_name != protobuf_c_empty_string) {
473+
free(resp->resp_scan_thread_result->entries[i]->network_name);
474+
}
401475
free(resp->resp_scan_thread_result->entries[i]);
402476
}
403477
free(resp->resp_scan_thread_result->entries);
@@ -450,13 +524,12 @@ esp_err_t network_prov_scan_handler(uint32_t session_id, const uint8_t *inbuf, s
450524

451525
network_scan_payload__init(&resp);
452526
ret = network_prov_scan_cmd_dispatcher(req, &resp, priv_data);
527+
resp.msg = req->msg + 1; /* Response is request + 1; set before error check so cleanup always runs correct case */
453528
if (ret != ESP_OK) {
454529
ESP_LOGE(TAG, "Command dispatcher error %d", ret);
455530
ret = ESP_FAIL;
456531
goto exit;
457532
}
458-
459-
resp.msg = req->msg + 1; /* Response is request + 1 */
460533
*outlen = network_scan_payload__get_packed_size(&resp);
461534
if (*outlen <= 0) {
462535
ESP_LOGE(TAG, "Invalid encoding for response");

0 commit comments

Comments
 (0)