Re: [PATCH] staging: rtl8723bs: fix missing shared-key auth challenge length check

[Manuel Ebner <manuelebner@mailbox.org>]

On Wed, 2026-07-08 at 11:43 +0300, Panagiotis Petrakopoulos wrote:
> The WEP shared-key authentication handlers use the challenge-text
> element's attacker-controlled length without checking it against the
> fixed 128-byte chg_txt buffer.

Plenty long and complex sentence. It took me a couple minutes to
understand (or misunderstand). Please split into a couple sentences.


That's my try in rewording (is it possible to drop 'shared key'?):

The WEP shared key authentication handlers use the challenge-text
element's length. This text and it's lenght are attacker-controlled.
The handler does not check the lenght against the fixed 128-byte
chg_txt buffer.


> In OnAuthClient() the length from rtw_get_ie() - up to 255 - is used
> to perform memcpy() into the 128-byte pmlmeinfo->chg_txt, so a
> malicious AP sending a malformed WLAN_EID_CHALLENGE element can
> overflow/underfill chg_txt by up to 127 bytes.

Again, but this one isn't as bad.

>  It is reachable over the
> air, before association, during shared-key authentication. In the case
> of an overflow, the driver can write out of bounds. In the case of an
> underfill, the driver can echo stale buffer memory. In OnAuth() a
> similar issue is observed. The driver compares a full 128 bytes
> regardless of the element's length, reading past a shorter element.
> 
> The challenge text is defined to be exactly 128 octets, which is
> already provided as the WLAN_AUTH_CHALLENGE_LEN define; require the
> element to be exactly that length in both handlers.

This is good.

> Fixes: 554c0a3abf21 ("staging: Add rtl8723bs sdio wifi driver")
I added the mentioned people in the commit to cc.

Thanks and
Reviewed-by: Manuel Ebner <manuelebner@mailbox.org>

> Cc: stable@vger.kernel.org
> Signed-off-by: Panagiotis Petrakopoulos <npetrakopoulos2003@gmail.com>
> ---
> Compile-tested only; I do not have RTL8723BS hardware to test the
> shared-key authentication path at runtime. The change only rejects
> challenge elements whose length differs from the spec-mandated 128
> bytes, so conforming peers are unaffected.
> 
>  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 a86d6f97cf02..13634d4e83d1 100644
> --- a/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
> +++ b/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
> @@ -787,7 +787,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 != WLAN_AUTH_CHALLENGE_LEN) {
>  				status = WLAN_STATUS_CHALLENGE_FAIL;
>  				goto auth_fail;
>  			}
> @@ -873,7 +873,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 != WLAN_AUTH_CHALLENGE_LEN)
>  				goto authclnt_fail;
>  
>  			memcpy(pmlmeinfo->chg_txt, p + 2, len);