Re: [PATCH v3] staging: rtl8723bs: fix remote heap info disclosure and OOB reads

[Luka Gejak]

On Wed Apr 15, 2026 at 3:50 PM CEST, Dan Carpenter wrote:
> On Wed, Apr 15, 2026 at 03:37:26PM +0200, luka.gejak@xxxxxxxxx wrote:
>> From: Luka Gejak <luka.gejak@xxxxxxxxx>
>> 
>> When building an association request frame, the driver iterates over
>> the ies received from the ap. In three places, the driver trusts the
>> attacker-controlled pIE->length without validating that it meets the
>> minimum expected size for the respective ie.
>> 
>> For WLAN_EID_HT_CAPABILITY, this causes an oob read of adjacent heap
>> memory which is then transmitted over the air (remote heap information
>> disclosure). For WLAN_EID_VENDOR_SPECIFIC, it causes two separate oob
>> reads: one when checking the 4-byte oui, and another when copying the
>> 14-byte wps ie.
>> 
>> Fix these issues by adding explicit length checks and returning a
>> failure if the length is insufficient. For HT_CAPABILITY, also clamp
>> the length passed to rtw_set_ie() to the struct size.
>> 
>> Fixes: 554c0a3abf21 ("staging: Add rtl8723bs sdio wifi driver")
>> Cc: stable@xxxxxxxxxxxxxxx
>> Signed-off-by: Luka Gejak <luka.gejak@xxxxxxxxx>
>> ---
>> Changes in v3:
>> - Switched to fail-fast handling for malformed IEs in issue_assocreq().
>> - Fixed HT capability path to use structure-sized output length in rtw_set_ie().
>> - Updated commit message to reflect all oob read cases.
>> 
>> Changes in v2:
>> - Refactored rtw_set_ie() alignment to follow "open parenthesis" style.
>> - Allowed the line length to exceed 100 characters for better readability as requested by Greg KH.
>> 
>>  drivers/staging/rtl8723bs/core/rtw_mlme_ext.c | 14 +++++++++++++-
>>  1 file changed, 13 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..3d44bc36532d 100644
>> --- a/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
>> +++ b/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
>> @@ -2929,6 +2929,9 @@ void issue_assocreq(struct adapter *padapter)
>>  
>>  		switch (pIE->element_id) {
>>  		case WLAN_EID_VENDOR_SPECIFIC:
>> +			if (pIE->length < 4)
>> +				goto exit;
>
> Oh huh.  I was more thinking about an upper bound, but yeah we need a
> both.  Anyway, what should the upper bound be?
>
> regards,
> dan carpenter

Hi Dan,
You are completely right, an upper bound check is necessary here as 
well. If the attacker provides a length that exceeds the remaining 
buffer size, the driver will read past the end of the received packet. 
I've added the upper bound checks at the beginning of the loop to ensure
both the ie header and its payload strictly fit within the remaining 
pmlmeinfo->network.ie_length. I have included this along with the 
lower-bound checks in v4.
Best regards,
Luka Gejak