From de85de7c5175c9742e756f5bb0852ce2256c6a4c Mon Sep 17 00:00:00 2001 From: "kapil.gupta" Date: Wed, 29 Apr 2020 12:52:23 +0530 Subject: [PATCH] wpa_supplicant: Fix some memleaks and invalid memory access Add changes to fix issues reported in clang analyzer --- .../src/esp_supplicant/esp_wpa2.c | 9 ++++ components/wpa_supplicant/src/wps/wps.c | 43 +++++++++---------- .../wpa_supplicant/src/wps/wps_registrar.c | 1 + 3 files changed, 30 insertions(+), 23 deletions(-) diff --git a/components/wpa_supplicant/src/esp_supplicant/esp_wpa2.c b/components/wpa_supplicant/src/esp_supplicant/esp_wpa2.c index 39a286af6..1d32cb7c3 100644 --- a/components/wpa_supplicant/src/esp_supplicant/esp_wpa2.c +++ b/components/wpa_supplicant/src/esp_supplicant/esp_wpa2.c @@ -761,6 +761,7 @@ static int eap_peer_sm_init(void) if (ret) { wpa_printf(MSG_ERROR, "eap_peer_blob_init failed\n"); os_free(sm); + vSemaphoreDelete(s_wpa2_data_lock); return ESP_FAIL; } @@ -769,6 +770,7 @@ static int eap_peer_sm_init(void) wpa_printf(MSG_ERROR, "eap_peer_config_init failed\n"); eap_peer_blob_deinit(sm); os_free(sm); + vSemaphoreDelete(s_wpa2_data_lock); return ESP_FAIL; } @@ -779,6 +781,7 @@ static int eap_peer_sm_init(void) eap_peer_blob_deinit(sm); eap_peer_config_deinit(sm); os_free(sm); + vSemaphoreDelete(s_wpa2_data_lock); return ESP_FAIL; } @@ -790,6 +793,12 @@ static int eap_peer_sm_init(void) xTaskCreate(wpa2_task, "wpa2T", WPA2_TASK_STACK_SIZE, NULL, 2, s_wpa2_task_hdl); s_wifi_wpa2_sync_sem = xSemaphoreCreateCounting(1, 0); if (!s_wifi_wpa2_sync_sem) { + vQueueDelete(s_wpa2_queue); + s_wpa2_queue = NULL; + eap_peer_blob_deinit(sm); + eap_peer_config_deinit(sm); + os_free(sm); + vSemaphoreDelete(s_wpa2_data_lock); wpa_printf(MSG_ERROR, "WPA2: failed create wifi wpa2 task sync sem"); return ESP_FAIL; } diff --git a/components/wpa_supplicant/src/wps/wps.c b/components/wpa_supplicant/src/wps/wps.c index b19e187a4..7067e194a 100644 --- a/components/wpa_supplicant/src/wps/wps.c +++ b/components/wpa_supplicant/src/wps/wps.c @@ -260,46 +260,43 @@ _out: * provisioning, -1 if wps_a is considered more like, or 0 if no preference */ int wps_ap_priority_compar(const struct wpabuf *wps_a, - const struct wpabuf *wps_b) + const struct wpabuf *wps_b) { - struct wps_parse_attr *attr_a, *attr_b; + struct wps_parse_attr *attr = NULL; int sel_a, sel_b; - int ret = 0; + int ret = 0; /* No preference */ - attr_a = (struct wps_parse_attr *)os_zalloc(sizeof(struct wps_parse_attr)); - attr_b = (struct wps_parse_attr *)os_zalloc(sizeof(struct wps_parse_attr)); + attr = os_zalloc(sizeof(*attr)); - if (attr_a == NULL || attr_b == NULL) { - ret = 0; - goto _out; + if (!attr) + return ret; + + if (wps_a == NULL || wps_parse_msg(wps_a, attr) < 0) { + ret = 1; + goto exit; } + sel_a = attr->selected_registrar && *(attr->selected_registrar) != 0; - if (wps_a == NULL || wps_parse_msg(wps_a, attr_a) < 0) - return 1; - if (wps_b == NULL || wps_parse_msg(wps_b, attr_b) < 0) - return -1; - - sel_a = attr_a->selected_registrar && *attr_a->selected_registrar != 0; - sel_b = attr_b->selected_registrar && *attr_b->selected_registrar != 0; + if (wps_b == NULL || wps_parse_msg(wps_b, attr) < 0) { + ret = -1; + goto exit; + } + sel_b = attr->selected_registrar && *(attr->selected_registrar) != 0; if (sel_a && !sel_b) { ret = -1; - goto _out; + goto exit; } if (!sel_a && sel_b) { ret = 1; - goto _out; + goto exit; } -_out: - if (attr_a) - os_free(attr_a); - if (attr_b) - os_free(attr_b); +exit: + os_free(attr); return ret; } - /** * wps_get_uuid_e - Get UUID-E from WPS IE * @msg: WPS IE contents from Beacon or Probe Response frame diff --git a/components/wpa_supplicant/src/wps/wps_registrar.c b/components/wpa_supplicant/src/wps/wps_registrar.c index e197dfdc0..b8778f2e5 100644 --- a/components/wpa_supplicant/src/wps/wps_registrar.c +++ b/components/wpa_supplicant/src/wps/wps_registrar.c @@ -1640,6 +1640,7 @@ int wps_build_cred(struct wps_data *wps, struct wpabuf *msg) if (random_get_bytes(r, sizeof(r)) < 0) return -1; os_free(wps->new_psk); + wps->new_psk = (u8 *)base64_encode(r, sizeof(r), &wps->new_psk_len); if (wps->new_psk == NULL) return -1; wps->new_psk_len--; /* remove newline */