brcmfmac: heap overflow in brcmf_notify_auth_frame_rx() on a short auth frame

[Maoyi Xie]

Hi all,

I think brcmf_notify_auth_frame_rx() in
drivers/net/wireless/broadcom/brcm80211/brcmfmac/cyw/core.c can overflow the
heap when the firmware reports a short external auth frame. I would
appreciate it if you could take a look.

The handler takes the frame length from the event, then allocates a buffer
for it.

	u32 mgmt_frame_len = e->datalen - sizeof(struct brcmf_rx_mgmt_data);
	...
	if (e->datalen < sizeof(*rxframe)) {
		...
		return -EINVAL;
	}
	...
	mgmt_frame = kzalloc(mgmt_frame_len, GFP_KERNEL);

The only length check is e->datalen >= sizeof(*rxframe). So mgmt_frame_len
can be anything from 0 up. The frame body is then copied with a length that
subtracts the management header offset.

	memcpy(&mgmt_frame->u, frame,
	       mgmt_frame_len - offsetof(struct ieee80211_mgmt, u));

offsetof(struct ieee80211_mgmt, u) is 24. If mgmt_frame_len is less than 24,
the subtraction wraps around as an unsigned value to a huge number. The
memcpy then runs far past the small kzalloc buffer. That is a heap overflow
driven by the frame the firmware passes up. A malicious or malfunctioning AP
can make the frame short during the external SAE auth exchange.

The p2p path in the same driver allocates with the header offset included,
so it does not have this shape.

I reproduced the overflow on 7.1-rc7. With mgmt_frame_len set below the 24
byte header offset, the subtracted length wraps to a huge value and the copy
faults.

  BUG: unable to handle page fault ... in memcpy_orig

A check that mgmt_frame_len is at least offsetof(struct ieee80211_mgmt, u)
before the copy would close it.

Does this look like a real bug to you, and is that the right place to bound
it? If so I am happy to send a proper patch with a Fixes tag and Cc stable.

Kaixuan Li and I found this together.

Thanks,
Maoyi
https://maoyixie.com/