Re: [PATCH v2] staging: rtl8723bs: bound WPS attribute copy in rtw_get_wps_attr_content()

[Greg Kroah-Hartman]

On Tue, Sep 01, 2026 at 11:12:26AM +0200, Jesus Olmos wrote:
> rtw_get_wps_attr_content() copies attr_len - 4 (the WPS attribute's 2-byte
> data-length field, up to 0xffff) from a WPS information element into the
> caller's buffer with no destination-size bound:
> 
> 	memcpy(buf_content, attr_ptr + 4, attr_len - 4);
> 
> The information element comes straight from a received beacon / probe
> response: collect_bss_info() copies the frame's IEs verbatim into
> bssid->ies, which reaches the scan queue, so attr_len is attacker
> controlled. rtw_cfg80211_inform_bss() and two sites in rtw_mlme_ext.c call
> this for WPS_ATTR_SELECTED_REGISTRAR with a one-byte destination (u8 sr /
> u8 selected_registrar), because that attribute is a single byte by spec. A
> frame that declares a longer Selected Registrar attribute therefore
> overflows the one-byte stack variable during a scan, which happens
> automatically (NetworkManager/iwd), giving an unauthenticated adjacent
> attacker a remote stack buffer overflow (at minimum a stack-protector
> panic).
> 
> Commit 1463ca3ec660 ("staging: rtl8723bs: fix OOB reads in rtw_get_sec_ie(),
> rtw_get_wapi_ie(), and rtw_get_wps_attr()") added a bounds check for the
> attribute header in rtw_get_wps_attr() but not for the attribute data
> length, and did not touch rtw_get_wps_attr_content(), so the copy remained
> both an out-of-bounds read of the attribute data and an out-of-bounds write
> of the destination.
> 
> Reject attributes that claim more data than the IE holds (fixing the
> out-of-bounds read and the latent memcpy(buf_attr, ...) in
> rtw_get_wps_attr()), give rtw_get_wps_attr_content() the destination buffer
> size, and clamp the copy to it.
> 
> Compute the attribute length in an unsigned int rather than u16: a declared
> data length of 0xfffc made (u16)(attr_data_len + 4) wrap to 0, which slipped
> past that bounds check and advanced the parser by zero, looping forever.
> 
> Found using mwemu (https://github.com/sha0coder/mwemu).
> 
> Fixes: 554c0a3abf21 ("staging: Add rtl8723bs sdio wifi driver")
> Cc: stable@xxxxxxxxxxxxxxx
> Assisted-by: Claude (Anthropic)
> Signed-off-by: Jesus Olmos <sha0@xxxxxxxxxxxxxxx>
> ---
> v2:
>  - Widen attr_len to unsigned int so a declared data length near 0xffff
>    cannot wrap; for 0xfffc the old u16 wrap defeated the bounds check and
>    made rtw_get_wps_attr() loop forever. Thanks Greg for spotting it.
>  - Add Assisted-by: tag for the AI-assisted analysis.
> 
> Build-tested as a module (x86_64 defconfig + CONFIG_RTL8723BS=m). Not tested
> on real hardware (I don't have an RTL8723BS device). The bug and the fix were
> found and checked by source review plus function-level emulation of
> rtw_get_wps_attr()/rtw_get_wps_attr_content() under mwemu: with a 0xfffc data
> length the pre-fix code spins forever (u16 wrap -> attr_ptr += 0) while the
> fix returns immediately, and the Selected Registrar overflow is clamped to the
> 1-byte destination.

Please see:
	https://lore.kernel.org/r/2026080354-skater-urgent-31b2@gregkh
for why I can't take this patch.

thanks,

greg k-h