Re: [PATCH] staging: rtl8723bs: validate HT capability IE length before use

[Ali Ahmet Memis <ali@iusegentoo.com>]

Please drop this one. Four of the five hunks are right, the first one is
not, and it is a regression rather than a fix.

The rtw_ap.c hunk is not an HT capability site at all:

	p = rtw_get_ie(ie + _BEACON_IE_OFFSET_,
		       WLAN_EID_SSID,
		       &ie_len,
		       (pbss_network->ie_length - _BEACON_IE_OFFSET_));
	if (p && ie_len >= sizeof(struct ieee80211_ht_cap)) {
		memcpy(pbss_network->ssid.ssid, (p + 2), ie_len);

That is the SSID, and I put an HT capability sized lower bound on it.
sizeof(struct ieee80211_ht_cap) is 26, SSIDs are 0 to 32 bytes, so every
SSID shorter than 26 characters now fails the test and the copy and the
ssid_length assignment are skipped. rtw_check_beacon_data() is the AP
setup path, so this breaks bringing up an AP with an ordinary SSID. I
should have checked which element each site fetches instead of assuming
all five were the same.

While looking at it, the check that site actually needs is the opposite
bound. rtw_get_ie() writes the raw IE length byte to *len and only limits
it against the remaining buffer:

	tmp = *(p + 1);
	if (i + 2 + tmp > limit)
		break;
	if (*p == index) {
		*len = tmp;

so ie_len can be up to 255, while the destination is

	struct ndis_802_11_ssid {
		u32  ssid_length;
		u8   ssid[32];
	};

and rtw_check_beacon_data() validates nothing but len <= MAX_IE_SZ before
that memcpy. An SSID element longer than 32 bytes overruns ssid[] inside
pmlmepriv->cur_network.network. It comes in through cfg80211 start_ap and
change_beacon, so it needs CAP_NET_ADMIN and a beacon that hostapd would
not normally build, but the bound is missing.

I will send a v2 with only the four HT capability sites, and the SSID
length check separately, since it is a different bug in the other
direction.

Sorry for the noise.

--
Ali