Re: Heap based overflow in Marvell Wifi mwifiex_cfg80211_start_ap function

Re: Heap based overflow in Marvell Wifi mwifiex_cfg80211_start_ap function

[Kalle Valo]

max chee <maxchee@outlook.com> writes:

> I found a possible heap-based overflow through code review in marvell wifi chip driver in Linux
> Kernel, allowing local users to cause a denial of service or possibly execute arbitrary code. Similar
> to CVE-2019-14814, I think that the bug can be triggered by sending crafted packet via netlink.
>
> I believe this belongs to the Linux Wireless mailing list
> (https://patchwork.kernel.org/project/linux-wireless/)

Please don't send HTML emails, our mailing lists drop them. I found this
from my spam folder just by accident.

> Description
>
>  
>
> ==========
>
>  
>
> [1]Heap Overflow in mwifiex_cfg80211_start_ap() function of Marvell Wifi Driver in Linux kernel
>
>  
>
>  
>
> The problem is inside mwifiex_cfg80211_start_ap() in
> drivers/net/wireless/marvell/mwifiex/cfg80211.c
>
>  
>
> There is a memcpy calls in this function to copy params->ssid without checking length. This
> would result in a heap overflow because params->ssid_len is from cfg80211_ap_settings which is
> from user space.
>
>  
>
>  
>
> Recommended Patch in drivers/net/wireless/marvell/mwifiex/cfg80211.c
>
>  
>
> =====
>
>  
>
> +                             if (ssid_len > IEEE80211_MAX_SSID_LEN) {
>
> +                             mwifiex_dbg(priv->adapter, ERROR, "invalid SSID - aborting\n");
>
> +                             return -EINVAL;
>
> +             }
>
>                 if (params->ssid && params->ssid_len) {
>
>                                 memcpy(bss_cfg->ssid.ssid, params->ssid, params->ssid_len);
>
>                                 bss_cfg->ssid.ssid_len = params->ssid_len;
>
>  
>
>  
>
>  
>
> Credit
>
>  
>
> ==========
>
>  
>
> This issue was discovered by Max Chee

Are you saying that cfg80211 does not check the SSID length from user
space? I would be very surprised about that.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: Heap based overflow in Marvell Wifi mwifiex_cfg80211_start_ap function

[Dan Carpenter]

This is a false positive.  The nla stuff has checking which is pretty
tricky for static analysis to handle.  Here is how that attribute is
defined.

net/wireless/nl80211.c
   544          [NL80211_ATTR_SSID] = { .type = NLA_BINARY,
   545                                  .len = IEEE80211_MAX_SSID_LEN },

When the NL80211_ATTR_SSID attr is parsed it's capped at
0-IEEE80211_MAX_SSID_LEN because of this.

regards,
dan carpenter