* [PATCH] sae: fix potential integer overflow
@ 2019-10-21 18:09 James Prestwood
0 siblings, 0 replies; only message in thread
From: James Prestwood @ 2019-10-21 18:09 UTC (permalink / raw)
To: iwd
[-- Attachment #1: Type: text/plain, Size: 2118 bytes --]
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 take the difference from 'hdr' and 'auth' to
find the true header length (either 24 or 28 bytes). Doing this lets us
simply check if len < 6 and reject since this would not be a valid auth
frame.
---
src/sae.c | 18 +++---------------
1 file changed, 3 insertions(+), 15 deletions(-)
diff --git a/src/sae.c b/src/sae.c
index 8f9425f1..0fb5f662 100644
--- a/src/sae.c
+++ b/src/sae.c
@@ -1021,25 +1021,13 @@ static int sae_rx_authenticate(struct auth_proto *ap,
goto reject;
}
- len -= sizeof(struct mmpdu_header);
+ len -= ((const void *)auth - (const void *)hdr);
- if (len < 4) {
- l_error("bad packet length");
+ if (len < 6) {
+ l_error("bad packet length %zu", len);
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;
-
ret = sae_verify_packet(sm, L_LE16_TO_CPU(auth->transaction_sequence),
L_LE16_TO_CPU(auth->status),
auth->ies, len - 6);
--
2.17.1
^ permalink raw reply related [flat|nested] only message in thread
only message in thread, other threads:[~2019-10-21 18:09 UTC | newest]
Thread overview: (only message) (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-10-21 18:09 [PATCH] sae: fix potential integer overflow James Prestwood
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.