[PATCH v2 0/2] wifi: libertas: fix OOB reads from firmware response fields

[PATCH v2 0/2] wifi: libertas: fix OOB reads from firmware response fields

[Tristan Madani]

From: Tristan Madani <tristan@talencesecurity.com>

Hi Johannes,

Note: this is a v2 resubmission. The original was sent via Gmail which
caused HTML rendering issues. This version uses git send-email for
proper plain-text formatting.

Two issues in libertas where firmware-controlled fields are used as
buffer offsets without validation:

Proposed fixes in the following patches.

Thanks,
Tristan

[PATCH v2 1/2] wifi: libertas: fix OOB read from firmware pkt_ptr offset in RX path

[Tristan Madani]

From: Tristan Madani <tristan@talencesecurity.com>

lbs_process_rxed_packet() uses the firmware-supplied pkt_ptr as an
offset into the skb data without validating that it falls within the
skb buffer bounds. A malicious pkt_ptr value causes out-of-bounds
memory access when the function subsequently reads ethernet header
fields from p_rx_pkt.

Add a bounds check to ensure pkt_ptr plus the minimum packet header
size does not exceed skb->len.

Fixes: e45d8e534b67 ("libertas: add support for Marvell SD8688 chip")
Signed-off-by: Tristan Madani <tristan@talencesecurity.com>
---
drivers/net/wireless/marvell/libertas/rx.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/net/wireless/marvell/libertas/rx.c b/drivers/net/wireless/marvell/libertas/rx.c
index XXXXXXX..XXXXXXX 100644
--- a/drivers/net/wireless/marvell/libertas/rx.c
+++ b/drivers/net/wireless/marvell/libertas/rx.c
@@ -75,6 +75,14 @@ int lbs_process_rxed_packet(struct lbs_private *priv, struct sk_buff *skb)
 	p_rx_pd = (struct rxpd *) skb->data;
+
+	if (le32_to_cpu(p_rx_pd->pkt_ptr) + sizeof(struct rxpackethdr) >
+	    skb->len) {
+		lbs_deb_rx("rx err: pkt_ptr %u beyond skb len %u\n",
+			   le32_to_cpu(p_rx_pd->pkt_ptr), skb->len);
+		ret = -EINVAL;
+		dev_kfree_skb(skb);
+		goto done;
+	}
 	p_rx_pkt = (struct rxpackethdr *) ((u8 *)p_rx_pd +
 		le32_to_cpu(p_rx_pd->pkt_ptr));
--
2.43.0

[PATCH v2 2/2] wifi: libertas: fix OOB read from firmware bssdescriptsize in scan response

[Tristan Madani]

From: Tristan Madani <tristan@talencesecurity.com>

The firmware-controlled bssdescriptsize field in lbs_ret_scan() is used
to compute the TSF descriptor position without validation against the
response buffer size. An inflated value causes out-of-bounds reads from
the 2312-byte response buffer into adjacent struct lbs_private members.

Add a check that bssdescriptsize fits within the response data.

Fixes: ff9fc791940f ("libertas: first stab at cfg80211 support")
Signed-off-by: Tristan Madani <tristan@talencesecurity.com>
---
drivers/net/wireless/marvell/libertas/cfg.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/net/wireless/marvell/libertas/cfg.c b/drivers/net/wireless/marvell/libertas/cfg.c
index XXXXXXX..XXXXXXX 100644
--- a/drivers/net/wireless/marvell/libertas/cfg.c
+++ b/drivers/net/wireless/marvell/libertas/cfg.c
@@ -555,6 +555,14 @@ static int lbs_ret_scan(struct lbs_private *priv, unsigned long dummy,

 	bsssize = get_unaligned_le16(&scanresp->bssdescriptsize);

+	if (bsssize > le16_to_cpu(resp->size) -
+	    sizeof(struct cmd_ds_802_11_scan_rsp)) {
+		lbs_deb_scan(
+			"scan response: bssdescriptsize %d exceeds response\n",
+			bsssize);
+		goto done;
+	}
+
 	lbs_deb_scan("scan response: %d BSSs (%d bytes); resp size %d bytes\n",
 			scanresp->nr_sets, bsssize, le16_to_cpu(resp->size));