[PATCH v4 2/2] staging: rtl8723bs: fix OOB reads in OnAssocRsp() IE parsing

[Alexandru Hossu]

Three out-of-bounds read paths in OnAssocRsp():

1. Missing minimum frame length check before fixed field reads.

   Before entering the IE loop the function reads capability, status,
   and AID from fixed offsets relative to pframe + WLAN_HDR_A3_LEN
   (at offsets +0, +2, and +4 respectively, so 6 bytes total).  There
   is no check that pkt_len is large enough to cover these fields.  A
   malicious AP can send a truncated Association Response frame shorter
   than WLAN_HDR_A3_LEN + 6 bytes, causing out-of-bounds reads and
   loading garbage into MLME state variables.

2. IE header and payload may extend past the packet end.

   The IE loop advances by pIE->length + 2 per iteration but only
   guards on i < pkt_len.  When the last IE has only one byte left in
   the frame, the loop reads pIE->length from pframe[pkt_len], one
   byte past the receive buffer.  Even when the header bytes are in
   bounds, pIE->length can point the data window past pkt_len, silently
   passing a truncated IE to the handler functions.

3. WMM OUI comparison reads 6 bytes past a possibly short IE payload.

   For WLAN_EID_VENDOR_SPECIFIC, the code calls memcmp(pIE->data,
   WMM_PARA_OUI, 6) without checking that pIE->length is at least 6.
   An attacker can craft a vendor-specific IE at the end of the frame
   with pIE->length smaller than 6.  The existing IE bounds check only
   confirms the declared payload fits within pkt_len, not that it is
   large enough for the 6-byte OUI comparison.

Fix all three:
  - Return _FAIL immediately if pkt_len < WLAN_HDR_A3_LEN + 6.
  - Add two guards in the IE loop: break if fewer than sizeof(*pIE)
    bytes remain, and break if the declared IE payload extends past
    pkt_len.
  - Guard the WMM OUI comparison with pIE->length >= 6.

Fixes: 554c0a3abf21 ("staging: Add rtl8723bs sdio wifi driver")
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: Alexandru Hossu <hossu.alexandru@xxxxxxxxx>
---
Changes in v4:
  - Add pkt_len < WLAN_HDR_A3_LEN + 6 check before reading the three
    fixed fields (capability, status, AID) to prevent OOB reads from
    truncated frames. Caught by sashiko review of v3.
  - Add pIE->length >= 6 guard before the 6-byte WMM OUI memcmp to
    prevent reading past a short IE payload. Caught by sashiko.

Changes in v2:
  - Add IE header bounds check: break if i + sizeof(*pIE) > pkt_len.
  - Add IE payload bounds check: break if the declared IE data extends
    past pkt_len.

 drivers/staging/rtl8723bs/core/rtw_mlme_ext.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c b/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
index 5f00fe282d1b..84cc814f069c 100644
--- a/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
+++ b/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
@@ -1379,6 +1379,9 @@ unsigned int OnAssocRsp(struct adapter *padapter, union recv_frame *precv_frame)
 
 	timer_delete_sync(&pmlmeext->link_timer);
 
+	if (pkt_len < WLAN_HDR_A3_LEN + 6)
+		return _FAIL;
+
 	/* status */
 	status = le16_to_cpu(*(__le16 *)(pframe + WLAN_HDR_A3_LEN + 2));
 	if (status > 0) {
@@ -1400,11 +1403,16 @@ unsigned int OnAssocRsp(struct adapter *padapter, union recv_frame *precv_frame)
 	/* to handle HT, WMM, rate adaptive, update MAC reg */
 	/* for not to handle the synchronous IO in the tasklet */
 	for (i = (6 + WLAN_HDR_A3_LEN); i < pkt_len;) {
+		if (i + sizeof(*pIE) > pkt_len)
+			break;
 		pIE = (struct ndis_80211_var_ie *)(pframe + i);
+		if (i + sizeof(*pIE) + pIE->length > pkt_len)
+			break;
 
 		switch (pIE->element_id) {
 		case WLAN_EID_VENDOR_SPECIFIC:
-			if (!memcmp(pIE->data, WMM_PARA_OUI, 6))	/* WMM */
+			if (pIE->length >= 6 &&
+			    !memcmp(pIE->data, WMM_PARA_OUI, 6))	/* WMM */
 				WMM_param_handler(padapter, pIE);
 			break;
 
-- 
2.53.0