[PATCH wireless] wifi: rtlwifi: fix OOB reads in IE parsing functions

[PATCH wireless] wifi: rtlwifi: fix OOB reads in IE parsing functions

[Tristan Madani]

From: Tristan Madani <tristan@talencesecurity.com>

rtl_find_ie() and rtl_find_221_ie() iterate over information elements
in beacon frames using a loop condition of `while (pos < end)`.  When
only one byte remains (pos == end - 1), the loop body accesses pos[1]
which reads one byte beyond the validated buffer boundary.

In rtl_find_ie(), pos[1] is read in the bounds check expression
`pos + 2 + pos[1] > end` before the result of that check can prevent
the access.

In rtl_find_221_ie(), the situation is worse: when the last byte
happens to be 0xDD (221, vendor-specific IE), the code accesses
pos[1] for the length, pos[2] for the data pointer, and passes both to
rtl_chk_vendor_ouisub() which performs memcmp() on the out-of-bounds
data.  The bounds check `pos + 2 + pos[1] > end` only appears after the
vendor IE has already been fully processed.

Fix both functions by tightening the loop condition to
`while (pos + 2 <= end)`, which ensures that at least the IE id (pos[0])
and length (pos[1]) bytes are within bounds before any access.
Additionally, in rtl_find_221_ie(), move the full IE bounds check before
the vendor-specific processing to prevent accessing IE data that extends
past the buffer.

Fixes: acd48572c396 ("rtlwifi: Change base routines for addition of rtl8192se and rtl8192de")
Cc: stable@vger.kernel.org
Signed-off-by: Tristan Madani <tristan@talencesecurity.com>
---
 drivers/net/wireless/realtek/rtlwifi/base.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtlwifi/base.c b/drivers/net/wireless/realtek/rtlwifi/base.c
index 9e98c01bb90e6..9671b9247b571 100644
--- a/drivers/net/wireless/realtek/rtlwifi/base.c
+++ b/drivers/net/wireless/realtek/rtlwifi/base.c
@@ -2373,7 +2373,7 @@ u8 *rtl_find_ie(u8 *data, unsigned int len, u8 ie)
 
 	pos = (u8 *)mgmt->u.beacon.variable;
 	end = data + len;
-	while (pos < end) {
+	while (pos + 2 <= end) {
 		if (pos + 2 + pos[1] > end)
 			return NULL;
 
@@ -2597,17 +2597,17 @@ static bool rtl_find_221_ie(struct ieee80211_hw *hw, u8 *data,
 
 	pos = (u8 *)mgmt->u.beacon.variable;
 	end = data + len;
-	while (pos < end) {
-		if (pos[0] == 221) {
+	while (pos + 2 <= end) {
+		if (pos + 2 + pos[1] > end)
+			return false;
+
+		if (pos[0] == 221 && pos[1] >= 3) {
 			vendor_ie.length = pos[1];
 			vendor_ie.octet = &pos[2];
 			if (rtl_chk_vendor_ouisub(hw, vendor_ie))
 				return true;
 		}
 
-		if (pos + 2 + pos[1] > end)
-			return false;
-
 		pos += 2 + pos[1];
 	}
 	return false;
-- 
2.47.3

RE: [PATCH wireless] wifi: rtlwifi: fix OOB reads in IE parsing functions

[Ping-Ke Shih]

Tristan Madani <tristmd@gmail.com> wrote:

[...]

> @@ -2597,17 +2597,17 @@ static bool rtl_find_221_ie(struct ieee80211_hw *hw, u8 *data,
> 
>         pos = (u8 *)mgmt->u.beacon.variable;
>         end = data + len;
> -       while (pos < end) {
> -               if (pos[0] == 221) {
> +       while (pos + 2 <= end) {
> +               if (pos + 2 + pos[1] > end)
> +                       return false;
> +
> +               if (pos[0] == 221 && pos[1] >= 3) {

I think the 3 is because length of OUI is 3 used by rtl_chk_vendor_ouisub().
Please leave a comment for that reason.

>                         vendor_ie.length = pos[1];
>                         vendor_ie.octet = &pos[2];
>                         if (rtl_chk_vendor_ouisub(hw, vendor_ie))
>                                 return true;
>                 }
> 
> -               if (pos + 2 + pos[1] > end)
> -                       return false;
> -
>                 pos += 2 + pos[1];
>         }
>         return false;
> --
> 2.47.3