Browse Source

qcacld-3.0: Check for minimum frame_len for action frames

In lim_process_action_frame and lim_process_action_frame_no_session,
The Rx frame pointer is directly casted to the action frame header
to find the Action frame category and action ID without validating
the minimum length of the frame. If the frame len is less than the
action frame header len, then OOB read would occur.

Check if frame_len is less than the size of action frame header len
and return if true.

Change-ID: Idf8ca7eeacdf57171d2850fe6317784911830aac
CRs-Fixed: 2253243
Vignesh Viswanathan 6 years ago
parent
commit
5f37b21cb6
1 changed files with 23 additions and 12 deletions
  1. 23 12
      core/mac/src/pe/lim/lim_process_action_frame.c

+ 23 - 12
core/mac/src/pe/lim/lim_process_action_frame.c

@@ -1808,11 +1808,18 @@ void lim_process_action_frame(tpAniSirGlobal mac_ctx,
 #endif
 	tpSirMacMgmtHdr mac_hdr = NULL;
 	int8_t rssi;
-	uint32_t frame_len;
+	uint32_t frame_len = WMA_GET_RX_PAYLOAD_LEN(rx_pkt_info);
 	tpSirMacVendorSpecificFrameHdr vendor_specific;
 	uint8_t oui[] = { 0x00, 0x00, 0xf0 };
-	tpSirMacVendorSpecificPublicActionFrameHdr pub_action;
 	uint8_t dpp_oui[] = { 0x50, 0x6F, 0x9A, 0x1A };
+	tpSirMacVendorSpecificPublicActionFrameHdr pub_action;
+
+	if (frame_len < sizeof(*action_hdr)) {
+		pe_debug("frame_len %d less than Action Frame Hdr size",
+			 frame_len);
+		return;
+	}
+
 #ifdef WLAN_FEATURE_11W
 	if (lim_is_robust_mgmt_action_frame(action_hdr->category) &&
 	   lim_drop_unprotected_action_frame(mac_ctx, session,
@@ -1820,8 +1827,6 @@ void lim_process_action_frame(tpAniSirGlobal mac_ctx,
 		return;
 #endif
 
-	frame_len = WMA_GET_RX_PAYLOAD_LEN(rx_pkt_info);
-
 	switch (action_hdr->category) {
 	case SIR_MAC_ACTION_QOS_MGMT:
 		if ((session->limQosEnabled) ||
@@ -2014,10 +2019,14 @@ void lim_process_action_frame(tpAniSirGlobal mac_ctx,
 	case SIR_MAC_ACTION_VENDOR_SPECIFIC_CATEGORY:
 		vendor_specific = (tpSirMacVendorSpecificFrameHdr) action_hdr;
 		mac_hdr = NULL;
-		frame_len = 0;
 
 		mac_hdr = WMA_GET_RX_MAC_HEADER(rx_pkt_info);
-		frame_len = WMA_GET_RX_PAYLOAD_LEN(rx_pkt_info);
+
+		if (frame_len < sizeof(*vendor_specific)) {
+			pe_debug("frame len %d less than Vendor Specific Hdr len",
+				 frame_len);
+			return;
+		}
 
 		/* Check if it is a vendor specific action frame. */
 		if (LIM_IS_STA_ROLE(session) &&
@@ -2058,7 +2067,6 @@ void lim_process_action_frame(tpAniSirGlobal mac_ctx,
 	break;
 	case SIR_MAC_ACTION_PUBLIC_USAGE:
 		mac_hdr = WMA_GET_RX_MAC_HEADER(rx_pkt_info);
-		frame_len = WMA_GET_RX_PAYLOAD_LEN(rx_pkt_info);
 
 		switch (action_hdr->actionID) {
 		case SIR_MAC_ACTION_EXT_CHANNEL_SWITCH_ID:
@@ -2069,7 +2077,7 @@ void lim_process_action_frame(tpAniSirGlobal mac_ctx,
 			pub_action =
 				(tpSirMacVendorSpecificPublicActionFrameHdr)
 				action_hdr;
-			if (frame_len < sizeof(pub_action)) {
+			if (frame_len < sizeof(*pub_action)) {
 				pe_debug("Received vendor specific public action frame of invalid len %d",
 					 frame_len);
 				return;
@@ -2155,10 +2163,8 @@ void lim_process_action_frame(tpAniSirGlobal mac_ctx,
 		break;
 	case SIR_MAC_ACTION_FST: {
 		tpSirMacMgmtHdr     hdr;
-		uint32_t            frame_len;
 
 		hdr = WMA_GET_RX_MAC_HEADER(rx_pkt_info);
-		frame_len = WMA_GET_RX_PAYLOAD_LEN(rx_pkt_info);
 
 		pe_debug("Received FST MGMT action frame");
 		/* Forward to the SME to HDD */
@@ -2181,7 +2187,6 @@ void lim_process_action_frame(tpAniSirGlobal mac_ctx,
 		case SIR_MAC_PDPA_GAS_COMEBACK_REQ:
 		case SIR_MAC_PDPA_GAS_COMEBACK_RSP:
 			mac_hdr = WMA_GET_RX_MAC_HEADER(rx_pkt_info);
-			frame_len = WMA_GET_RX_PAYLOAD_LEN(rx_pkt_info);
 			rssi = WMA_GET_RX_RSSI_NORMALIZED(rx_pkt_info);
 			lim_send_sme_mgmt_frame_ind(mac_ctx,
 				mac_hdr->fc.subType, (uint8_t *) mac_hdr,
@@ -2246,6 +2251,12 @@ void lim_process_action_frame_no_session(tpAniSirGlobal pMac, uint8_t *pBd)
 
 	pe_debug("Received an Action frame -- no session");
 
+	if (frame_len < sizeof(*action_hdr)) {
+		pe_debug("frame_len %d less than action frame header len",
+			 frame_len);
+		return;
+	}
+
 	switch (action_hdr->category) {
 	case SIR_MAC_ACTION_PUBLIC_USAGE:
 		switch (action_hdr->actionID) {
@@ -2254,7 +2265,7 @@ void lim_process_action_frame_no_session(tpAniSirGlobal pMac, uint8_t *pBd)
 				(tpSirMacVendorSpecificPublicActionFrameHdr)
 				action_hdr;
 
-			if (frame_len < sizeof(vendor_specific)) {
+			if (frame_len < sizeof(*vendor_specific)) {
 				pe_debug("Received vendor specific public action frame of invalid len %d",
 					 frame_len);
 				return;