Re: {Disarmed} Two potential 1-byte BOF in cfg80211.c of driver MWIFIEX

[Greg KH <gregkh@linuxfoundation.org>]

On Sun, Sep 15, 2024 at 06:12:21PM +0000, Sherlock Fang wrote:
> Dear Security Team,
> 
> 
> I have used static code analysis tools to scan the Linux Kernel of the latest build, and with collaborative help from Prof. Yueqi Chen, we have been able to manually confirm the presence of two 1-byte BOF in drivers/net/wireless/marvell/mwifiex/cfg80211.c
> 
> At both line https://github.com/torvalds/linux/blob/master/drivers/net/wireless/marvell/mwifiex/cfg80211.c#L2679 and https://github.com/torvalds/linux/blob/master/drivers/net/wireless/marvell/mwifiex/cfg80211.c#L2777, the calls to
> 
> memcpy(&priv->vs_ie[i].ie, ie, sizeof(*ie) + ie->len);
> 
> are theoretically possible to cause a buffer overflow since the destination buffer is an array of bytes with a size defined by MWIFIEX_MAX_VSIE_LEN<https://github.com/torvalds/linux/blob/master/drivers/net/wireless/marvell/mwifiex/ioctl.h#L416> of 256 bytes, which is the maximum amount of data that can safely be stored in priv->vs_ie[i].ie<https://github.com/torvalds/linux/blob/master/drivers/net/wireless/marvell/mwifiex/main.h#L483>. The size parameter "sizeof(*ie) + ie->len" theoretically has the maximum value of "2 + 255 = 257" given that len is of type u8 which has upper limit of 255, and sizeof(*ie) is equivalent to sizeof(struct ieee_types_header<https://github.com/torvalds/linux/blob/master/drivers/net/wireless/marvell/mwifiex/main.h#L343>), that is just the size of two u8 field (two bytes).
> 
> Our suggested fix would be to add a check before calling memcpy:
> 
> If (sizeof(*ie) + ie->len > sizeof(&priv->vs_ie[i].ie)) {
>       continue;
> }
> 
> memcpy(&priv->vs_ie[i].ie, ie, sizeof(*ie) + ie->len);

Great, can you submit a real patch for this so that it can be reviewed
and accepted if ok?

Also, you are sure that the data being copied could really be that big,
right?  Where does it come from?

thanks,

greg k-h