Merge branch 'bugfix/eap_client_crash_v4.1' into 'release/v4.1'

wpa_supplicant: Fix memory corruption (v4.1)

See merge request espressif/esp-idf!17131

components/wpa_supplicant/src/crypto/tls_mbedtls.c

@@ -119,21 +119,18 @@ static int tls_mbedtls_read(void *ctx, unsigned char *buf, size_t len)
 	struct tls_connection *conn = (struct tls_connection *)ctx;
 	struct tls_data *data = &conn->tls_io_data;
 	struct wpabuf *local_buf;
-	size_t data_len = len;
 
-	if (data->in_data == NULL) {
+	if (data->in_data == NULL || len > wpabuf_len(data->in_data)) {
+		/* We don't have suffient buffer available for read */
+		wpa_printf(MSG_INFO, "len=%zu not available in input", len);
 		return MBEDTLS_ERR_SSL_WANT_READ;
 	}
 
-	if (len > wpabuf_len(data->in_data)) {
-		wpa_printf(MSG_ERROR, "don't have suffient data\n");
-		data_len = wpabuf_len(data->in_data);
-	}
-
-	os_memcpy(buf, wpabuf_head(data->in_data), data_len);
+	os_memcpy(buf, wpabuf_head(data->in_data), len);
 	/* adjust buffer */
 	if (len < wpabuf_len(data->in_data)) {
-		local_buf = wpabuf_alloc_copy(wpabuf_head(data->in_data) + len,
+		/* TODO optimize this operation */
+		local_buf = wpabuf_alloc_copy(wpabuf_mhead_u8(data->in_data) + len,
 					      wpabuf_len(data->in_data) - len);
 		wpabuf_free(data->in_data);
 		data->in_data = local_buf;
@@ -142,7 +139,7 @@ static int tls_mbedtls_read(void *ctx, unsigned char *buf, size_t len)
 		data->in_data = NULL;
 	}
 
-	return data_len;
+	return len;
 }
 
 static int set_pki_context(tls_context_t *tls, const struct tls_connection_params *cfg)
@@ -558,6 +555,7 @@ struct wpabuf * tls_connection_handshake(void *tls_ctx,
 {
 	tls_context_t *tls = conn->tls;
 	int ret = 0;
+	struct wpabuf *resp;
 
 	/* data freed by sender */
 	conn->tls_io_data.out_data = NULL;
@@ -591,7 +589,9 @@ struct wpabuf * tls_connection_handshake(void *tls_ctx,
 	}
 
 end:
-	return conn->tls_io_data.out_data;
+	resp = conn->tls_io_data.out_data;
+	conn->tls_io_data.out_data = NULL;
+	return resp;
 }
 
 struct wpabuf * tls_connection_server_handshake(void *tls_ctx,
@@ -608,10 +608,12 @@ struct wpabuf * tls_connection_encrypt(void *tls_ctx,
 				       struct tls_connection *conn,
 				       const struct wpabuf *in_data)
 {
+	struct wpabuf *resp;
+	size_t ret;
+
 	/* Reset dangling pointer */
 	conn->tls_io_data.out_data = NULL;
-
-	ssize_t ret = mbedtls_ssl_write(&conn->tls->ssl,
+	ret = mbedtls_ssl_write(&conn->tls->ssl,
 			(unsigned char*) wpabuf_head(in_data),  wpabuf_len(in_data));
 
 	if (ret < wpabuf_len(in_data)) {
@@ -619,27 +621,50 @@ struct wpabuf * tls_connection_encrypt(void *tls_ctx,
 			   __func__, __LINE__);
 	}
 
-	return conn->tls_io_data.out_data;
+	resp = conn->tls_io_data.out_data;
+	conn->tls_io_data.out_data = NULL;
+	return resp;
 }
 
 
-struct wpabuf * tls_connection_decrypt(void *tls_ctx,
-		struct tls_connection *conn,
-		const struct wpabuf *in_data)
+struct wpabuf *tls_connection_decrypt(void *tls_ctx,
+				      struct tls_connection *conn,
+				      const struct wpabuf *in_data)
 {
-	unsigned char buf[1200];
+#define MAX_PHASE2_BUFFER 1536
+	struct wpabuf *out = NULL;
 	int ret;
-	conn->tls_io_data.in_data = wpabuf_dup(in_data);
-	ret = mbedtls_ssl_read(&conn->tls->ssl, buf, 1200);
-	if (ret < 0) {
-		wpa_printf(MSG_ERROR, "%s:%d, not able to write whole data",
-				__func__, __LINE__);
+	unsigned char *buf = os_malloc(MAX_PHASE2_BUFFER);
+
+	if (!buf) {
 		return NULL;
 	}
+	/* Reset dangling output buffer before setting data, data was freed by caller */
+	conn->tls_io_data.out_data = NULL;
 
-	struct wpabuf *out = wpabuf_alloc_copy(buf, ret);
+	conn->tls_io_data.in_data = wpabuf_dup(in_data);
+
+	if (!conn->tls_io_data.in_data) {
+		goto cleanup;
+	}
+	ret = mbedtls_ssl_read(&conn->tls->ssl, buf, MAX_PHASE2_BUFFER);
+	if (ret < 0) {
+		wpa_printf(MSG_ERROR, "%s:%d, not able to read data",
+				__func__, __LINE__);
+		goto cleanup;
+	}
+	out = wpabuf_alloc_copy(buf, ret);
+cleanup:
+	/* there may be some error written in output buffer */
+	if (conn->tls_io_data.out_data) {
+		os_free(conn->tls_io_data.out_data);
+		conn->tls_io_data.out_data = NULL;
+	}
+
+	os_free(buf);
 
 	return out;
+#undef MAX_PHASE2_BUFFER
 }
 
 

components/wpa_supplicant/src/eap_peer/eap_peap.c

@@ -1042,10 +1042,9 @@ continue_req:
 }
 
 
-static struct wpabuf *
-eap_peap_process(struct eap_sm *sm, void *priv,
-		 struct eap_method_ret *ret,
-		 const struct wpabuf *reqData)
+static struct wpabuf * eap_peap_process(struct eap_sm *sm, void *priv,
+					struct eap_method_ret *ret,
+					const struct wpabuf *reqData)
 {
 	const struct eap_hdr *req;
 	size_t left;
@@ -1096,6 +1095,14 @@ eap_peap_process(struct eap_sm *sm, void *priv,
 						  data->peap_version, id, pos,
 						  left, &resp);
 
+		if (res < 0) {
+			wpa_printf(MSG_DEBUG,
+				   "EAP-PEAP: TLS processing failed");
+			ret->methodState = METHOD_DONE;
+			ret->decision = DECISION_FAIL;
+			return resp;
+		}
+
 		if (tls_connection_established(sm->ssl_ctx, data->ssl.conn)) {
 			char label[24];
 			wpa_printf(MSG_DEBUG, "EAP-PEAP: TLS done, proceed to Phase 2");

components/wpa_supplicant/src/esp_supplicant/esp_wpa2.c

@@ -412,7 +412,8 @@ int eap_sm_process_request(struct eap_sm *sm, struct wpabuf *reqData)
         return ESP_FAIL;
     }
 
-    if (ehdr->identifier == sm->current_identifier) {
+    if (ehdr->identifier == sm->current_identifier &&
+        sm->lastRespData != NULL) {
         /*Retransmit*/
         resp = sm->lastRespData;
         goto send_resp;
@@ -486,7 +487,10 @@ build_nak:
 send_resp:
     if (resp == NULL) {
         wpa_printf(MSG_ERROR, "Response build fail, return.");
-        return ESP_FAIL;
+        wpabuf_free(sm->lastRespData);
+        sm->lastRespData = resp;
+        wpa2_set_eap_state(WPA2_ENT_EAP_STATE_FAIL);
+        return WPA2_ENT_EAP_STATE_FAIL;
     }
     ret = eap_sm_send_eapol(sm, resp);
     if (ret == ESP_OK) {