From 3f2b558f5772ee1e5b11535b4353e726feee7552 Mon Sep 17 00:00:00 2001
From: James Prestwood <prestwoj@gmail.com>
Date: Mon, 21 Oct 2019 14:01:58 -0700
Subject: [PATCH] sae: fix potential integer overflow

If an authentication frame of length <= 5 is sent sae will overflow an
integer. The original cause of this was due to incorrectly using the
sizeof(struct mmpdu_header). The header can be either 24 or 28 bytes
depending on fc.order. sizeof does not account for this so 28 is always
the calculated length.

This, in addition to hostapd not including a group number when rejecting,
cause this erroneous length calculation to be worked around as seen in
the removed comment. The comment is still valid (and described again
in another location) but the actual check for len == 4 is not correct.

To fix this we now rely on mpdu_validate to check that the authentication
frame is valid, and then subtract the actual header length using
mmpdu_header_len rather than sizeof. Doing this lets us also remove the
length check since it was validated previously.
---
 src/sae.c | 28 ++++++++++------------------
 1 file changed, 10 insertions(+), 18 deletions(-)

diff --git a/src/sae.c b/src/sae.c
index 232a63b9..d5e88128 100644
--- a/src/sae.c
+++ b/src/sae.c
@@ -692,6 +692,9 @@ static int sae_verify_nothing(struct sae_sm *sm, uint16_t transaction,
 	if (status != 0)
 		return -EBADMSG;
 
+	if (len < 2)
+		return -EBADMSG;
+
 	/* reject with unsupported group */
 	if (l_get_le16(frame) != sm->group) {
 		sae_reject_authentication(sm,
@@ -912,6 +915,9 @@ static int sae_verify_confirmed(struct sae_sm *sm, uint16_t trans,
 	if (sm->sync > SAE_SYNC_MAX)
 		return -EBADMSG;
 
+	if (len < 2)
+		return -EBADMSG;
+
 	/* frame shall be silently discarded */
 	if (l_get_le16(frame) != sm->group)
 		return 0;
@@ -947,6 +953,9 @@ static bool sae_verify_accepted(struct sae_sm *sm, uint16_t trans,
 	if (sm->sync > SAE_SYNC_MAX)
 		return false;
 
+	if (len < 2)
+		return false;
+
 	sc = l_get_le16(frame);
 
 	/*
@@ -1024,24 +1033,7 @@ static int sae_rx_authenticate(struct auth_proto *ap,
 		goto reject;
 	}
 
-	len -= sizeof(struct mmpdu_header);
-
-	if (len < 4) {
-		l_error("bad packet length");
-		goto reject;
-	}
-
-	/*
-	 * TODO: Hostapd seems to not include the group number when rejecting
-	 * with an unsupported group, which violates the spec. This means our
-	 * len == 4, but we can still recover this connection by renegotiating
-	 * a new group. Because of this we need to special case this status
-	 * code, as well as add the check in the verify function to allow for
-	 * this missing group number.
-	 */
-	if (len == 4 && L_LE16_TO_CPU(auth->status) !=
-				MMPDU_STATUS_CODE_UNSUPP_FINITE_CYCLIC_GROUP)
-		goto reject;
+	len -= mmpdu_header_len(hdr);
 
 	ret = sae_verify_packet(sm, L_LE16_TO_CPU(auth->transaction_sequence),
 					L_LE16_TO_CPU(auth->status),