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

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

[Alexandru Hossu]

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.

Changes in v6:
- Add hansg@kernel.org to Cc (original driver author; accidentally
  omitted from the v5 series)
- Patch content unchanged from initial submission

 drivers/staging/rtl8723bs/core/rtw_mlme_ext.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c b/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
index 5f00fe282d1b..90f27665667a 100644
--- a/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
+++ b/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
@@ -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 v6 2/2] staging: rtl8723bs: fix missing frame length checks in OnAuthClient

[Alexandru Hossu]

OnAuthClient() accesses pframe without first verifying that pkt_len is
large enough to contain a valid 802.11 management frame header:

- get_da(pframe) reads bytes 4-9, requiring pkt_len >= 10
- GetPrivacy(pframe) reads the FC field at bytes 0-1

Additionally, when pkt_len < WLAN_HDR_A3_LEN + _AUTH_IE_OFFSET_ the
unsigned subtraction passed to rtw_get_ie() wraps around, causing it
to scan well past the end of the buffer.

Add an early check against WLAN_HDR_A3_LEN before any pframe access,
and a second check against WLAN_HDR_A3_LEN + offset + 6 after computing
offset to guard the seq/status reads and the rtw_get_ie() call.

Suggested-by: Dan Carpenter <error27@gmail.com>
Fixes: 554c0a3abf21 ("staging: Add rtl8723bs sdio wifi driver")
Cc: stable@vger.kernel.org
Cc: hansg@kernel.org
Reviewed-by: Dan Carpenter <error27@gmail.com>
Reviewed-by: Luka Gejak <luka.gejak@linux.dev>
Signed-off-by: Alexandru Hossu <hossu.alexandru@gmail.com>
---
Changes in v6:
- Add hansg@kernel.org to Cc (original driver author; accidentally
  omitted from the v5 series)

Changes in v5:
- Resend as 2/2 in two-patch series at maintainer request
- Add Reviewed-by from Dan Carpenter and Luka Gejak

Changes in v4:
- Replace incorrect Reported-by with Suggested-by: Dan spotted the
  missing length check during code review of the heap overflow fix;
  he did not file a separate bug report
- Add missing version changelog; correct subject line version number
  (previous submission was mislabeled as v2 despite being v3)

Changes in v3:
- Add first check against WLAN_HDR_A3_LEN before any pframe access
  to also guard get_da() and prevent unsigned subtraction wrap
- Rename subject to "fix missing frame length checks"

Changes in v2:
- Add single length check after computing offset to guard the
  seq/status field reads

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

diff --git a/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c b/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
index 90f27665667a..884cd39ec756 100644
--- a/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
+++ b/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
@@ -860,6 +860,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 +872,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 v6 1/2] staging: rtl8723bs: fix heap overflow in OnAuthClient shared key path

[Luka Gejak]

On Wed Apr 15, 2026 at 11:45 AM CEST, 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.
>
> Changes in v6:
> - Add hansg@kernel.org to Cc (original driver author; accidentally
>   omitted from the v5 series)
> - Patch content unchanged from initial submission
>
>  drivers/staging/rtl8723bs/core/rtw_mlme_ext.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c b/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
> index 5f00fe282d1b..90f27665667a 100644
> --- a/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
> +++ b/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
> @@ -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);

LGTM.

Reviewed-by: Luka Gejak <luka.gejak@linux.dev>

Best regards,
Luka Gejak

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