[PATCH v2] wifi: mwifiex: Fix OOB and integer underflow in mwifiex_process_mgmt_packet

[PATCH v2] wifi: mwifiex: Fix OOB and integer underflow in mwifiex_process_mgmt_packet

[pinkperfect]

In outside functions have checked upper limit of rx_pkt_length,
in mwifiex_process_mgmt_packet should make sure rx_pkt_length not underflow
and make sure skb->len big enough to avoid OOB access.

Signed-off-by: pinkperfect <pinkperfect2021@gmail.com>
---
 drivers/net/wireless/marvell/mwifiex/util.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/wireless/marvell/mwifiex/util.c b/drivers/net/wireless/marvell/mwifiex/util.c
index 94c2d219835d..31e1a82883e4 100644
--- a/drivers/net/wireless/marvell/mwifiex/util.c
+++ b/drivers/net/wireless/marvell/mwifiex/util.c
@@ -399,6 +399,11 @@ mwifiex_process_mgmt_packet(struct mwifiex_private *priv,
 
 	pkt_len = le16_to_cpu(rx_pd->rx_pkt_length);
 
+	if (pkt_len < sizeof(struct ieee80211_hdr) || skb->len < pkt_len) {
+		mwifiex_dbg(priv->adapter, ERROR, "invalid rx_pkt_length");
+		return -1;
+	}
+
 	ieee_hdr = (void *)skb->data;
 	if (ieee80211_is_mgmt(ieee_hdr->frame_control)) {
 		if (mwifiex_parse_mgmt_packet(priv, (u8 *)ieee_hdr,
-- 
2.25.1

Re: [PATCH v2] wifi: mwifiex: Fix OOB and integer underflow in mwifiex_process_mgmt_packet

[pinkperfect]

Hi, this vulnerability has been reported to and discussed with chromeos teams,
the detail analysis, see comments in below code:
mwifiex_process_sta_rx_packet makes sure rx_pkt_offset + rx_pkt_length <= skb->len
In mwifiex_process_mgmt_packet:

        rx_pd = (struct rxpd *)skb->data;

        // skb->len -= rx_pkt_offset, skb->len == rx_pkt_length
        skb_pull(skb, le16_to_cpu(rx_pd->rx_pkt_offset));
        // skb->len == rx_pkt_length - 2, if set rx_pkt_length == 4, skb->len == 2
        skb_pull(skb, sizeof(pkt_len));

        pkt_len = le16_to_cpu(rx_pd->rx_pkt_length);

        //skip..

        // now skb->len == 2, skb->data + 24 is oob from skb buffer
        // skb->data + 30 is oob from skb buffer
        // pkt_len == 4, so underflow
        memmove(skb->data + sizeof(struct ieee80211_hdr_3addr),
                skb->data + sizeof(struct ieee80211_hdr),
                pkt_len - sizeof(struct ieee80211_hdr));

On MT8173 chromebook, the arm64 memmove.S / memcpy.S code logical
cause memove(dst, src, -x) a possible exploitable oob write vulnerability
not only a unexploitable crash

Re: [PATCH v2] wifi: mwifiex: Fix OOB and integer underflow in mwifiex_process_mgmt_packet

[Jakub Kicinski]

On Mon, 10 Jul 2023 15:01:30 +0000 pinkperfect wrote:
> Hi, this vulnerability has been reported to and discussed with chromeos teams,
> the detail analysis, see comments in below code:
> mwifiex_process_sta_rx_packet makes sure rx_pkt_offset + rx_pkt_length <= skb->len
> In mwifiex_process_mgmt_packet:
> 
>         rx_pd = (struct rxpd *)skb->data;
> 
>         // skb->len -= rx_pkt_offset, skb->len == rx_pkt_length
>         skb_pull(skb, le16_to_cpu(rx_pd->rx_pkt_offset));
>         // skb->len == rx_pkt_length - 2, if set rx_pkt_length == 4, skb->len == 2
>         skb_pull(skb, sizeof(pkt_len));
> 
>         pkt_len = le16_to_cpu(rx_pd->rx_pkt_length);
> 
>         //skip..
> 
>         // now skb->len == 2, skb->data + 24 is oob from skb buffer
>         // skb->data + 30 is oob from skb buffer
>         // pkt_len == 4, so underflow
>         memmove(skb->data + sizeof(struct ieee80211_hdr_3addr),
>                 skb->data + sizeof(struct ieee80211_hdr),
>                 pkt_len - sizeof(struct ieee80211_hdr));
> 
> On MT8173 chromebook, the arm64 memmove.S / memcpy.S code logical
> cause memove(dst, src, -x) a possible exploitable oob write vulnerability
> not only a unexploitable crash

Oh, didn't see the v2, please address the comments I just sent to v1.