diff mbox

[1/1] 802.1X: validate input before pointer

Message ID 1503011668-9350-1-git-send-email-michael-dev@fami-braun.de
State Accepted
Headers show

Commit Message

michael-dev Aug. 17, 2017, 11:14 p.m. UTC 
ieee802_1x_kay_decode_mkpdu calls ieee802_1x_mka_i_in_peerlist before
body_len has been checked on all segments.

ieee802_1x_kay_decode_mkpdu and ieee802_1x_mka_i_in_peerlist might
continue and thus underflow left_len even if it finds left_len to small
(or before checking).

Additionally, ieee802_1x_mka_dump_peer_body might perform out of bound
reads in this case.

Fix this by checking left_len and aborting if too small early.

Signed-off-by: Michael Braun <michael-dev@fami-braun.de>
---
 src/pae/ieee802_1x_kay.c | 23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

Comments

Jouni Malinen Sept. 25, 2017, 8:38 p.m. UTC | #1 
On Fri, Aug 18, 2017 at 01:14:28AM +0200, Michael Braun wrote:
> ieee802_1x_kay_decode_mkpdu calls ieee802_1x_mka_i_in_peerlist before
> body_len has been checked on all segments.
> 
> ieee802_1x_kay_decode_mkpdu and ieee802_1x_mka_i_in_peerlist might
> continue and thus underflow left_len even if it finds left_len to small
> (or before checking).
> 
> Additionally, ieee802_1x_mka_dump_peer_body might perform out of bound
> reads in this case.
> 
> Fix this by checking left_len and aborting if too small early.

Thanks, applied.
diff mbox

Patch

diff --git a/src/pae/ieee802_1x_kay.c b/src/pae/ieee802_1x_kay.c
index d3c880d..a919dfe 100644
--- a/src/pae/ieee802_1x_kay.c
+++ b/src/pae/ieee802_1x_kay.c
@@ -965,21 +965,19 @@  ieee802_1x_mka_i_in_peerlist(struct ieee802_1x_mka_participant *participant,
 		body_len = get_mka_param_body_len(hdr);
 		body_type = get_mka_param_body_type(hdr);
 
-		if (body_type != MKA_LIVE_PEER_LIST &&
-		    body_type != MKA_POTENTIAL_PEER_LIST)
-			continue;
-
-		ieee802_1x_mka_dump_peer_body(
-			(struct ieee802_1x_mka_peer_body *)pos);
-
-		if (left_len < (MKA_HDR_LEN + body_len + DEFAULT_ICV_LEN)) {
+		if (left_len < (MKA_HDR_LEN + MKA_ALIGN_LENGTH(body_len) + DEFAULT_ICV_LEN)) {
 			wpa_printf(MSG_ERROR,
 				   "KaY: MKA Peer Packet Body Length (%zu bytes) is less than the Parameter Set Header Length (%zu bytes) + the Parameter Set Body Length (%zu bytes) + %d bytes of ICV",
 				   left_len, MKA_HDR_LEN,
-				   body_len, DEFAULT_ICV_LEN);
-			continue;
+				   MKA_ALIGN_LENGTH(body_len),
+				   DEFAULT_ICV_LEN);
+			return FALSE;
 		}
 
+		if (body_type != MKA_LIVE_PEER_LIST &&
+		    body_type != MKA_POTENTIAL_PEER_LIST)
+			continue;
+
 		if ((body_len % 16) != 0) {
 			wpa_printf(MSG_ERROR,
 				   "KaY: MKA Peer Packet Body Length (%zu bytes) should be a multiple of 16 octets",
@@ -987,6 +985,9 @@  ieee802_1x_mka_i_in_peerlist(struct ieee802_1x_mka_participant *participant,
 			continue;
 		}
 
+		ieee802_1x_mka_dump_peer_body(
+			(struct ieee802_1x_mka_peer_body *)pos);
+
 		for (i = 0; i < body_len;
 		     i += sizeof(struct ieee802_1x_mka_peer_id)) {
 			const struct ieee802_1x_mka_peer_id *peer_mi;
@@ -3019,7 +3020,7 @@  static int ieee802_1x_kay_decode_mkpdu(struct ieee802_1x_kay *kay,
 				   "KaY: MKA Peer Packet Body Length (%zu bytes) is less than the Parameter Set Header Length (%zu bytes) + the Parameter Set Body Length (%zu bytes) + %d bytes of ICV",
 				   left_len, MKA_HDR_LEN,
 				   body_len, DEFAULT_ICV_LEN);
-			continue;
+			return -1;
 		}
 
 		if (handled[body_type])