All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Jesus Olmos <sha0@badchecksum.net>
Cc: linux-staging@lists.linux.dev, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] staging: rtl8723bs: bound WPS attribute copy in rtw_get_wps_attr_content()
Date: Tue, 1 Sep 2026 10:16:21 +0200	[thread overview]
Message-ID: <2026090133-postwar-kebab-83f0@gregkh> (raw)
In-Reply-To: <20260901072249.366750-1-sha0@badchecksum.net>

On Tue, Sep 01, 2026 at 09:22:49AM +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.
> 
> Found using mwemu (https://github.com/sha0coder/mwemu).

So shouldn't this have an Assisted-by: tag?

And have you tested this on real hardware?

Also, this looks a bit wrong, you have an endless loop in this patch :(

thanks,

greg k-h

  reply	other threads:[~2026-09-01  8:16 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-01  7:22 [PATCH] staging: rtl8723bs: bound WPS attribute copy in rtw_get_wps_attr_content() Jesus Olmos
2026-09-01  8:16 ` Greg Kroah-Hartman [this message]
2026-09-01  9:12 ` [PATCH v2] " Jesus Olmos
2026-09-01  9:20   ` Greg Kroah-Hartman
2026-09-20 19:49   ` [PATCH v2] staging: rtl8723bs: fix stack overflow in rtw_get_wps_attr_content Jesus Olmos

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2026090133-postwar-kebab-83f0@gregkh \
    --to=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-staging@lists.linux.dev \
    --cc=sha0@badchecksum.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.