Re: [PATCH v6 1/2] staging: rtl8723bs: fix heap overflow in OnAuthClient shared key path

Re: [PATCH v6 1/2] staging: rtl8723bs: fix heap overflow in OnAuthClient shared key path

[Greg KH]

On Wed, Apr 15, 2026 at 11:45:04AM +0200, Alexandru Hossu wrote:
> rtw_get_ie() returns the raw IE length from the received frame, which
> can be up to 255. This length is used directly in memcpy() into
> chg_txt[128] with no bounds check, allowing a heap overflow of up to
> 127 bytes when a rogue AP sends an Auth seq=2 frame with a Challenge
> Text IE longer than 128 bytes.
> 
> IEEE 802.11 mandates the Challenge Text element carries exactly 128
> bytes of challenge data. Reject any element whose length field does not
> match sizeof(pmlmeinfo->chg_txt) (128).
> 
> Fixes: 554c0a3abf21 ("staging: Add rtl8723bs sdio wifi driver")
> Cc: stable@vger.kernel.org
> Cc: hansg@kernel.org
> Reviewed-by: Dan Carpenter <error27@gmail.com>
> Signed-off-by: Alexandru Hossu <hossu.alexandru@gmail.com>
> ---
> Apologies for the version numbering confusion across previous iterations.

Please address the review comments found here in your next version:
	https://sashiko.dev/#/patchset/20260415094505.1115208-1-hossu.alexandru@gmail.com

thanks,

greg k-h

[PATCH v7 0/2] staging: rtl8723bs: fix OOB reads in OnAuth() and OnAuthClient()

[Alexandru Hossu]

v7, addressing the sashiko review comments on v6.

Regarding hardware: I do not have rtl8723bs hardware available.  The
patches in this series are derived from static analysis of the code,
cross-checking against the 802.11 spec, and reviewing the patterns
already in use elsewhere in the same driver.

This series fixes authentication frame handling in the rtl8723bs driver.

Patch 1/2 fixes heap overflows in the Challenge Text IE paths of both
OnAuthClient() (STA mode) and OnAuth() (AP mode): the IE length field
from the received frame was used without checking it equals 128, the
fixed size mandated by IEEE 802.11.

Patch 2/2 adds frame length guards before the first direct pframe
dereferences in both OnAuth() and OnAuthClient().  Without these checks,
a frame shorter than WLAN_HDR_A3_LEN bytes causes out-of-bounds reads
before any IE parsing even begins.  Two additional guards cover the
algorithm/sequence fields in OnAuth() and the seq/status fields in
OnAuthClient(), which are read at variable offsets past the 802.11 header.

OnAssocRsp() was already fixed in a separate series.

What changed in v7:

Patch 1/2:
  - No code changes from v6; dropping Reviewed-by: Dan Carpenter because
    patch 2/2 changes code from the reviewed version.

Patch 2/2:
  - Add frame length checks for OnAuth(): guard before GetAddr2Ptr
    (len < WLAN_HDR_A3_LEN) and guard before algorithm/seq reads
    (len < WLAN_HDR_A3_LEN + offset + 4).
  - Correct commit message: remove incorrect claim that rtw_get_ie()
    unsigned underflow causes OOB scan; rtw_get_ie() uses signed int
    limit and returns NULL immediately when limit < 2, so the wrapped
    value is caught before any scan occurs.

Alexandru Hossu (2):
  staging: rtl8723bs: fix Challenge Text IE length checks in
    OnAuthClient() and OnAuth()
  staging: rtl8723bs: fix missing frame length checks in OnAuth() and
    OnAuthClient()

 drivers/staging/rtl8723bs/core/rtw_mlme_ext.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

-- 
2.53.0

[PATCH v7 1/2] staging: rtl8723bs: fix Challenge Text IE length checks in OnAuthClient() and OnAuth()

[Alexandru Hossu]

Two functions process Challenge Text IEs without verifying that the IE
length matches the 128-byte buffer:

1. OnAuthClient() shared key path (STA mode).

   rtw_get_ie() returns the raw IE length from the received frame,
   which can be up to 255.  This length is used directly in memcpy()
   into chg_txt[128] with no bounds check, allowing a heap overflow of
   up to 127 bytes when a rogue AP sends an Auth seq=2 frame with a
   Challenge Text IE longer than 128 bytes.

2. OnAuth() sequence 3 path (AP mode).

   When a STA completes shared-key authentication, OnAuth() calls
   rtw_get_ie() to find the Challenge Text IE, checks only that the
   IE is present and has nonzero length, then calls
   memcmp((p + 2), pstat->chg_txt, 128).  If a rogue STA sends a
   Challenge Text IE shorter than 128 bytes, memcmp reads past the
   end of the IE payload into adjacent packet data, causing an
   out-of-bounds read.

IEEE 802.11 mandates the Challenge Text element carries exactly 128
bytes of challenge data.  Add len != sizeof(pmlmeinfo->chg_txt) and
ie_len != sizeof(pstat->chg_txt) guards to reject any element whose
length field does not match.

Fixes: 554c0a3abf21 ("staging: Add rtl8723bs sdio wifi driver")
Cc: stable@vger.kernel.org
Signed-off-by: Alexandru Hossu <hossu.alexandru@gmail.com>
---
Changes in v7:
  - No code changes from v6; dropping Reviewed-by: Dan Carpenter because
    patch 2/2 changes code from the reviewed version.

 drivers/staging/rtl8723bs/core/rtw_mlme_ext.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c b/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
index 5f00fe282d1b..dd3c94d314d8 100644
--- a/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
+++ b/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
@@ -802,7 +802,7 @@ unsigned int OnAuth(struct adapter *padapter, union recv_frame *precv_frame)
 			p = rtw_get_ie(pframe + WLAN_HDR_A3_LEN + 4 + _AUTH_IE_OFFSET_, WLAN_EID_CHALLENGE, (int *)&ie_len,
 					len - WLAN_HDR_A3_LEN - _AUTH_IE_OFFSET_ - 4);
 
-			if (!p || ie_len <= 0) {
+			if (!p || ie_len != sizeof(pstat->chg_txt)) {
 				status = WLAN_STATUS_CHALLENGE_FAIL;
 				goto auth_fail;
 			}
@@ -891,7 +891,7 @@ unsigned int OnAuthClient(struct adapter *padapter, union recv_frame *precv_fram
 			p = rtw_get_ie(pframe + WLAN_HDR_A3_LEN + _AUTH_IE_OFFSET_, WLAN_EID_CHALLENGE, (int *)&len,
 				pkt_len - WLAN_HDR_A3_LEN - _AUTH_IE_OFFSET_);
 
-			if (!p)
+			if (!p || len != sizeof(pmlmeinfo->chg_txt))
 				goto authclnt_fail;
 
 			memcpy(pmlmeinfo->chg_txt, p + 2, len);
-- 
2.53.0

[PATCH v7 2/2] staging: rtl8723bs: fix missing frame length checks in OnAuth() and OnAuthClient()

[Alexandru Hossu]

Four out-of-bounds read paths caused by missing frame length guards:

1. OnAuth() reads GetAddr2Ptr (pframe + 10) without verifying the frame
   is at least WLAN_HDR_A3_LEN bytes long.

   The first operation on pframe after the AP-state guard is
   GetAddr2Ptr(pframe), which reads 6 bytes at offset 10..15 (Addr2).
   If the received frame is shorter than WLAN_HDR_A3_LEN (24 bytes),
   this reads past the end of the frame buffer.  Add:
     if (len < WLAN_HDR_A3_LEN) goto auth_fail;

2. OnAuth() reads the algorithm and sequence fields at pframe +
   WLAN_HDR_A3_LEN + offset + {0,2} without verifying that those
   offsets are within the frame.

   offset is 0 for an open-system frame and 4 for a WEP-encapsulated
   frame.  The reads at offset+0 and offset+2 are both 2-byte, so the
   last byte accessed is at WLAN_HDR_A3_LEN + offset + 3.  A crafted
   short frame causes an out-of-bounds read.  Add:
     if (len < WLAN_HDR_A3_LEN + offset + 4) goto auth_fail;

3. OnAuthClient() calls get_da(pframe) without verifying the frame is
   at least WLAN_HDR_A3_LEN bytes long.

   get_da() inspects the ToDs and FrDs bits in Frame Control (bytes
   0..1) and returns either Addr1 (bytes 4..9) or Addr3 (bytes 16..21).
   A frame shorter than WLAN_HDR_A3_LEN (24 bytes) causes an
   out-of-bounds read in either case.  Add:
     if (pkt_len < WLAN_HDR_A3_LEN) goto authclnt_fail;

4. OnAuthClient() reads the sequence field at pframe + WLAN_HDR_A3_LEN
   + offset + 2 and the status field at offset + 4 without verifying
   those offsets are within the frame.

   offset is 0 for open-system and 4 for WEP.  The status read at
   offset+4 is 2 bytes, so the last byte accessed is at
   WLAN_HDR_A3_LEN + offset + 5.  Add:
     if (pkt_len < WLAN_HDR_A3_LEN + offset + 6) goto authclnt_fail;

Note: a previous version of this patch claimed that the signed/unsigned
mismatch in the rtw_get_ie() limit parameter caused an out-of-bounds
scan when pkt_len < WLAN_HDR_A3_LEN + _AUTH_IE_OFFSET_.  This is
incorrect: rtw_get_ie() declares its limit as signed int, so the
wrapped unsigned value is reinterpreted as a large negative number,
which is immediately caught by the if (limit < 2) return NULL; guard
inside rtw_get_ie().  The actual out-of-bounds reads are the four
direct pframe dereferences listed above.

OnAssocRsp() was already fixed by a separate series.

Fixes: 554c0a3abf21 ("staging: Add rtl8723bs sdio wifi driver")
Cc: stable@vger.kernel.org
Signed-off-by: Alexandru Hossu <hossu.alexandru@gmail.com>
---
Changes in v7:
  - Add frame length checks for OnAuth(): guard before GetAddr2Ptr (len <
    WLAN_HDR_A3_LEN) and guard before algorithm/seq reads (len <
    WLAN_HDR_A3_LEN + offset + 4) (sashiko review of v6).
  - Correct commit message: remove incorrect claim that rtw_get_ie()
    unsigned underflow causes OOB scan; rtw_get_ie() uses signed int
    limit and returns NULL when limit < 2 (sashiko review of v6).

Changes in v6:
  - Add frame length checks for OnAuthClient(): guard before get_da()
    (pkt_len < WLAN_HDR_A3_LEN) and guard before seq/status reads
    (pkt_len < WLAN_HDR_A3_LEN + offset + 6).
  - Correct commit message: OnAssocRsp() was already fixed in a
    separate series.

 drivers/staging/rtl8723bs/core/rtw_mlme_ext.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c b/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
index dd3c94d314d8..b42eab61d8a8 100644
--- a/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
+++ b/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
@@ -687,6 +687,9 @@ unsigned int OnAuth(struct adapter *padapter, union recv_frame *precv_frame)
 	if ((pmlmeinfo->state&0x03) != WIFI_FW_AP_STATE)
 		return _FAIL;
 
+	if (len < WLAN_HDR_A3_LEN)
+		goto auth_fail;
+
 	sa = GetAddr2Ptr(pframe);
 
 	auth_mode = psecuritypriv->dot11AuthAlgrthm;
@@ -709,6 +712,9 @@ unsigned int OnAuth(struct adapter *padapter, union recv_frame *precv_frame)
 		offset = 4;
 	}
 
+	if (len < WLAN_HDR_A3_LEN + offset + 4)
+		goto auth_fail;
+
 	algorithm = le16_to_cpu(*(__le16 *)((SIZE_PTR)pframe + WLAN_HDR_A3_LEN + offset));
 	seq	= le16_to_cpu(*(__le16 *)((SIZE_PTR)pframe + WLAN_HDR_A3_LEN + offset + 2));
 
@@ -860,6 +866,9 @@ unsigned int OnAuthClient(struct adapter *padapter, union recv_frame *precv_fram
 	u8 *pframe = precv_frame->u.hdr.rx_data;
 	uint pkt_len = precv_frame->u.hdr.len;
 
+	if (pkt_len < WLAN_HDR_A3_LEN)
+		goto authclnt_fail;
+
 	/* check A1 matches or not */
 	if (memcmp(myid(&(padapter->eeprompriv)), get_da(pframe), ETH_ALEN))
 		return _SUCCESS;
@@ -869,6 +878,9 @@ unsigned int OnAuthClient(struct adapter *padapter, union recv_frame *precv_fram
 
 	offset = (GetPrivacy(pframe)) ? 4 : 0;
 
+	if (pkt_len < WLAN_HDR_A3_LEN + offset + 6)
+		goto authclnt_fail;
+
 	seq	= le16_to_cpu(*(__le16 *)((SIZE_PTR)pframe + WLAN_HDR_A3_LEN + offset + 2));
 	status	= le16_to_cpu(*(__le16 *)((SIZE_PTR)pframe + WLAN_HDR_A3_LEN + offset + 4));
 
-- 
2.53.0

Re: [PATCH v7 0/2] staging: rtl8723bs: fix OOB reads in OnAuth() and OnAuthClient()

[Greg KH]

On Tue, May 05, 2026 at 11:13:14PM +0200, Alexandru Hossu wrote:
> v7, addressing the sashiko review comments on v6.

Some more comments on your patch 2/2:
	https://sashiko.dev/#/patchset/20260505211316.3837020-1-hossu.alexandru@gmail.com

thanks,

greg k-h