[PATCH v5 0/5] staging: rtl8723bs: fix multiple security vulnerabilities

Delene Tchio Romuald posted 5 patches 2 months ago
There is a newer version of this series
.../staging/rtl8723bs/core/rtw_ieee80211.c    | 70 +++++++++++++------
drivers/staging/rtl8723bs/core/rtw_recv.c     | 65 ++++++++++-------
drivers/staging/rtl8723bs/core/rtw_security.c |  6 ++
3 files changed, 92 insertions(+), 49 deletions(-)
[PATCH v5 0/5] staging: rtl8723bs: fix multiple security vulnerabilities
Posted by Delene Tchio Romuald 2 months ago
This series fixes five remotely-triggerable memory safety issues in
the rtl8723bs driver. All of them are reachable from the air by an
attacker within WiFi radio range, without authentication, via
crafted management or data frames:

  1. Heap buffer overflow in recvframe_defrag() when reassembling
     fragmented frames whose total payload exceeds the receive
     buffer capacity.
  2. Integer underflow in TKIP MIC verification when a frame is
     shorter than the sum of header, IV, ICV and MIC sizes.
  3. Out-of-bounds read in portctrl() when a non-EAPOL frame is
     shorter than the 802.11 header + IV + LLC + ether_type.
  4. Out-of-bounds reads in three IE walkers (rtw_get_wapi_ie(),
     rtw_get_sec_ie(), rtw_get_wps_ie()) due to missing validation
     of the TLV length byte and of the byte ranges touched by the
     subsequent memcmp() calls.
  5. Integer underflow in rtw_wep_decrypt() when a WEP frame is
     shorter than the header + IV + ICV.

Each patch was found by code review and is not tested on hardware.

Changes since v4:
 - Patch 1/5: collapse the five identical cleanup sites in
   recvframe_defrag() into a single out_err label (Dan Carpenter).
 - Patch 3/5: return NULL directly on the short-frame and
   non-EAPOL error paths instead of staging the result through
   prtnframe (Dan Carpenter).
 - Patch 4/5: in addition to the outer TLV length check, add an
   inner bound check before each memcmp() so that the OUI read at
   offset 6 (WAPI) or offset 2 (WPA/WPS) stays inside the declared
   element (Dan Carpenter).
 - Patch 5/5: tighten the length check to also cover the 4-byte
   ICV, so that the subsequent crc32_le(payload, length - 4) call
   cannot underflow length - 4.
 - Patches 1/5, 3/5, 4/5 and 5/5 lost Luka Gejak's Reviewed-by
   because the code changed; patch 2/5 carries it unchanged.

Changes since v3:
 - All patches: add Fixes: tag pointing at the driver import and
   add Cc: stable per Dan Carpenter.

Changes since v2:
 - Sent as numbered series with cover letter.

Changes since v1:
 - Rebased on staging-next.

Delene Tchio Romuald (5):
  staging: rtl8723bs: fix heap buffer overflow in recvframe_defrag()
  staging: rtl8723bs: fix integer underflow in TKIP MIC verification
  staging: rtl8723bs: fix out-of-bounds read in portctrl()
  staging: rtl8723bs: fix out-of-bounds reads in IE parsing functions
  staging: rtl8723bs: fix negative length in WEP decryption

 .../staging/rtl8723bs/core/rtw_ieee80211.c    | 70 +++++++++++++------
 drivers/staging/rtl8723bs/core/rtw_recv.c     | 65 ++++++++++-------
 drivers/staging/rtl8723bs/core/rtw_security.c |  6 ++
 3 files changed, 92 insertions(+), 49 deletions(-)


base-commit: bf9c95f3eeefb7fc4b4a6380cc23f1dca744e379
--
2.43.0
Re: [PATCH v5 0/5] staging: rtl8723bs: fix multiple security vulnerabilities
Posted by Dan Carpenter 1 month, 4 weeks ago
Sorry, we're really strict on the "no unrelated changes" rule.  Otherwise
it looks good.  Thanks!

regards,
dan carpenter