[PATCH] crypto: Check if crypto_bignum_to_bin is successful

Andrei Otcheretianski andrei.otcheretianski at intel.com
Mon Dec 5 05:31:17 PST 2022


From: Micha Hashkes <micha.hashkes at intel.com>

Return value of crypto_bignum_to_bin() wasn't always checked, resulting
in potential access to uninitialized values. Fix it, as some analyzers
complain about it.

Signed-off-by: Andrei Otcheretianski <andrei.otcheretianski at intel.com>
Signed-off-by: Micha Hashkes <micha.hashkes at intel.com>
---
 src/common/sae.c                |  4 +++-
 src/crypto/crypto_openssl.c     | 14 ++++++++------
 src/eap_common/eap_pwd_common.c | 22 +++++++++++++++++++---
 src/eap_peer/eap_pwd.c          | 33 ++++++++++++++++++++++++++-------
 src/eap_server/eap_server_pwd.c | 32 +++++++++++++++++++++++++-------
 5 files changed, 81 insertions(+), 24 deletions(-)

diff --git a/src/common/sae.c b/src/common/sae.c
index e597bfc1ac..03972f75ae 100644
--- a/src/common/sae.c
+++ b/src/common/sae.c
@@ -1606,7 +1606,9 @@ static int sae_derive_keys(struct sae_data *sae, const u8 *k)
 	 * (commit-scalar + peer-commit-scalar) mod r part as a bit string by
 	 * zero padding it from left to the length of the order (in full
 	 * octets). */
-	crypto_bignum_to_bin(tmp, val, sizeof(val), sae->tmp->order_len);
+	if (crypto_bignum_to_bin(tmp, val,
+				 sizeof(val), sae->tmp->order_len) < 0)
+		goto fail;
 	wpa_hexdump(MSG_DEBUG, "SAE: PMKID", val, SAE_PMKID_LEN);
 
 #ifdef CONFIG_SAE_PK
diff --git a/src/crypto/crypto_openssl.c b/src/crypto/crypto_openssl.c
index 31d9d6131a..fe070e37c5 100644
--- a/src/crypto/crypto_openssl.c
+++ b/src/crypto/crypto_openssl.c
@@ -2449,14 +2449,16 @@ int crypto_ec_point_to_bin(struct crypto_ec *e,
 	    EC_POINT_get_affine_coordinates(e->group, (EC_POINT *) point,
 					    x_bn, y_bn, e->bnctx)) {
 		if (x) {
-			crypto_bignum_to_bin((struct crypto_bignum *) x_bn,
-					     x, len, len);
+			ret = crypto_bignum_to_bin((struct crypto_bignum *) x_bn,
+						   x, len, len);
 		}
-		if (y) {
-			crypto_bignum_to_bin((struct crypto_bignum *) y_bn,
-					     y, len, len);
+		if (ret != -1 && y) {
+			ret = crypto_bignum_to_bin((struct crypto_bignum *) y_bn,
+						   y, len, len);
 		}
-		ret = 0;
+
+		if (ret > 0)
+			ret = 0;
 	}
 
 	BN_clear_free(x_bn);
diff --git a/src/eap_common/eap_pwd_common.c b/src/eap_common/eap_pwd_common.c
index 0e3a7b2b35..b1a2bd506d 100644
--- a/src/eap_common/eap_pwd_common.c
+++ b/src/eap_common/eap_pwd_common.c
@@ -356,9 +356,20 @@ int compute_keys(EAP_PWD_group *grp, const struct crypto_bignum *k,
 		return -1;
 	}
 	eap_pwd_h_update(hash, (const u8 *) ciphersuite, sizeof(u32));
-	crypto_bignum_to_bin(peer_scalar, cruft, order_len, order_len);
+	if (crypto_bignum_to_bin(peer_scalar, cruft, order_len,
+				 order_len) < 0) {
+		os_free(cruft);
+		return -1;
+	}
+
 	eap_pwd_h_update(hash, cruft, order_len);
-	crypto_bignum_to_bin(server_scalar, cruft, order_len, order_len);
+
+	if (crypto_bignum_to_bin(server_scalar, cruft, order_len,
+				 order_len) < 0) {
+		os_free(cruft);
+		return -1;
+	}
+
 	eap_pwd_h_update(hash, cruft, order_len);
 	eap_pwd_h_final(hash, &session_id[1]);
 
@@ -368,7 +379,12 @@ int compute_keys(EAP_PWD_group *grp, const struct crypto_bignum *k,
 		os_free(cruft);
 		return -1;
 	}
-	crypto_bignum_to_bin(k, cruft, prime_len, prime_len);
+
+	if (crypto_bignum_to_bin(k, cruft, prime_len, prime_len) < 0) {
+		os_free(cruft);
+		return -1;
+	}
+
 	eap_pwd_h_update(hash, cruft, prime_len);
 	os_free(cruft);
 	eap_pwd_h_update(hash, confirm_peer, SHA256_MAC_LEN);
diff --git a/src/eap_peer/eap_pwd.c b/src/eap_peer/eap_pwd.c
index 605feb24f8..97f4dd216f 100644
--- a/src/eap_peer/eap_pwd.c
+++ b/src/eap_peer/eap_pwd.c
@@ -666,7 +666,10 @@ eap_pwd_perform_commit_exchange(struct eap_sm *sm, struct eap_pwd_data *data,
 	 * sufficiently smaller than the prime or order might need pre-pending
 	 * with zeros.
 	 */
-	crypto_bignum_to_bin(data->my_scalar, scalar, order_len, order_len);
+	if (crypto_bignum_to_bin(data->my_scalar, scalar, order_len,
+				 order_len) < 0)
+		goto fin;
+
 	if (crypto_ec_point_to_bin(data->grp->group, data->my_element, element,
 				   element + prime_len) != 0) {
 		wpa_printf(MSG_INFO, "EAP-PWD (peer): point assignment fail");
@@ -742,7 +745,9 @@ eap_pwd_perform_confirm_exchange(struct eap_sm *sm, struct eap_pwd_data *data,
 	 * zero the memory each time because this is mod prime math and some
 	 * value may start with a few zeros and the previous one did not.
 	 */
-	crypto_bignum_to_bin(data->k, cruft, prime_len, prime_len);
+	if (crypto_bignum_to_bin(data->k, cruft, prime_len, prime_len) < 0)
+		goto fin;
+
 	eap_pwd_h_update(hash, cruft, prime_len);
 
 	/* server element: x, y */
@@ -755,7 +760,10 @@ eap_pwd_perform_confirm_exchange(struct eap_sm *sm, struct eap_pwd_data *data,
 	eap_pwd_h_update(hash, cruft, prime_len * 2);
 
 	/* server scalar */
-	crypto_bignum_to_bin(data->server_scalar, cruft, order_len, order_len);
+	if (crypto_bignum_to_bin(data->server_scalar, cruft, order_len,
+				 order_len) < 0)
+		goto fin;
+
 	eap_pwd_h_update(hash, cruft, order_len);
 
 	/* my element: x, y */
@@ -768,7 +776,10 @@ eap_pwd_perform_confirm_exchange(struct eap_sm *sm, struct eap_pwd_data *data,
 	eap_pwd_h_update(hash, cruft, prime_len * 2);
 
 	/* my scalar */
-	crypto_bignum_to_bin(data->my_scalar, cruft, order_len, order_len);
+	if (crypto_bignum_to_bin(data->my_scalar, cruft, order_len,
+				 order_len) < 0)
+		goto fin;
+
 	eap_pwd_h_update(hash, cruft, order_len);
 
 	/* the ciphersuite */
@@ -796,7 +807,9 @@ eap_pwd_perform_confirm_exchange(struct eap_sm *sm, struct eap_pwd_data *data,
 		goto fin;
 
 	/* k */
-	crypto_bignum_to_bin(data->k, cruft, prime_len, prime_len);
+	if (crypto_bignum_to_bin(data->k, cruft, prime_len, prime_len) < 0)
+		goto fin;
+
 	eap_pwd_h_update(hash, cruft, prime_len);
 
 	/* my element */
@@ -809,7 +822,10 @@ eap_pwd_perform_confirm_exchange(struct eap_sm *sm, struct eap_pwd_data *data,
 	eap_pwd_h_update(hash, cruft, prime_len * 2);
 
 	/* my scalar */
-	crypto_bignum_to_bin(data->my_scalar, cruft, order_len, order_len);
+	if (crypto_bignum_to_bin(data->my_scalar, cruft, order_len,
+				 order_len) < 0)
+		goto fin;
+
 	eap_pwd_h_update(hash, cruft, order_len);
 
 	/* server element: x, y */
@@ -822,7 +838,10 @@ eap_pwd_perform_confirm_exchange(struct eap_sm *sm, struct eap_pwd_data *data,
 	eap_pwd_h_update(hash, cruft, prime_len * 2);
 
 	/* server scalar */
-	crypto_bignum_to_bin(data->server_scalar, cruft, order_len, order_len);
+	if (crypto_bignum_to_bin(data->server_scalar, cruft, order_len,
+				 order_len) < 0)
+		goto fin;
+
 	eap_pwd_h_update(hash, cruft, order_len);
 
 	/* the ciphersuite */
diff --git a/src/eap_server/eap_server_pwd.c b/src/eap_server/eap_server_pwd.c
index 81cddca607..824705df6b 100644
--- a/src/eap_server/eap_server_pwd.c
+++ b/src/eap_server/eap_server_pwd.c
@@ -293,7 +293,9 @@ static void eap_pwd_build_commit_req(struct eap_sm *sm,
 	/* We send the element as (x,y) followed by the scalar */
 	element = wpabuf_put(data->outbuf, 2 * prime_len);
 	scalar = wpabuf_put(data->outbuf, order_len);
-	crypto_bignum_to_bin(data->my_scalar, scalar, order_len, order_len);
+	if (crypto_bignum_to_bin(data->my_scalar, scalar, order_len, order_len) < 0)
+		goto fin;
+
 	if (crypto_ec_point_to_bin(data->grp->group, data->my_element, element,
 				   element + prime_len) < 0) {
 		wpa_printf(MSG_INFO, "EAP-PWD (server): point assignment "
@@ -349,7 +351,9 @@ static void eap_pwd_build_confirm_req(struct eap_sm *sm,
 	 *
 	 * First is k
 	 */
-	crypto_bignum_to_bin(data->k, cruft, prime_len, prime_len);
+	if (crypto_bignum_to_bin(data->k, cruft, prime_len, prime_len) < 0)
+		goto fin;
+
 	eap_pwd_h_update(hash, cruft, prime_len);
 
 	/* server element: x, y */
@@ -362,7 +366,10 @@ static void eap_pwd_build_confirm_req(struct eap_sm *sm,
 	eap_pwd_h_update(hash, cruft, prime_len * 2);
 
 	/* server scalar */
-	crypto_bignum_to_bin(data->my_scalar, cruft, order_len, order_len);
+	if (crypto_bignum_to_bin(data->my_scalar, cruft, order_len,
+				 order_len) < 0)
+		goto fin;
+
 	eap_pwd_h_update(hash, cruft, order_len);
 
 	/* peer element: x, y */
@@ -375,7 +382,10 @@ static void eap_pwd_build_confirm_req(struct eap_sm *sm,
 	eap_pwd_h_update(hash, cruft, prime_len * 2);
 
 	/* peer scalar */
-	crypto_bignum_to_bin(data->peer_scalar, cruft, order_len, order_len);
+	if (crypto_bignum_to_bin(data->peer_scalar, cruft, order_len,
+				 order_len) < 0)
+		goto fin;
+
 	eap_pwd_h_update(hash, cruft, order_len);
 
 	/* ciphersuite */
@@ -785,7 +795,9 @@ eap_pwd_process_confirm_resp(struct eap_sm *sm, struct eap_pwd_data *data,
 		goto fin;
 
 	/* k */
-	crypto_bignum_to_bin(data->k, cruft, prime_len, prime_len);
+	if (crypto_bignum_to_bin(data->k, cruft, prime_len, prime_len) < 0)
+		goto fin;
+
 	eap_pwd_h_update(hash, cruft, prime_len);
 
 	/* peer element: x, y */
@@ -798,7 +810,10 @@ eap_pwd_process_confirm_resp(struct eap_sm *sm, struct eap_pwd_data *data,
 	eap_pwd_h_update(hash, cruft, prime_len * 2);
 
 	/* peer scalar */
-	crypto_bignum_to_bin(data->peer_scalar, cruft, order_len, order_len);
+	if (crypto_bignum_to_bin(data->peer_scalar, cruft, order_len,
+				 order_len) < 0)
+		goto fin;
+
 	eap_pwd_h_update(hash, cruft, order_len);
 
 	/* server element: x, y */
@@ -811,7 +826,10 @@ eap_pwd_process_confirm_resp(struct eap_sm *sm, struct eap_pwd_data *data,
 	eap_pwd_h_update(hash, cruft, prime_len * 2);
 
 	/* server scalar */
-	crypto_bignum_to_bin(data->my_scalar, cruft, order_len, order_len);
+	if (crypto_bignum_to_bin(data->my_scalar, cruft, order_len,
+				 order_len) < 0)
+		goto fin;
+
 	eap_pwd_h_update(hash, cruft, order_len);
 
 	/* ciphersuite */
-- 
2.25.1




More information about the Hostap mailing list