[PATCH 15/25] WNM: Add candidate list to BSS transition response

[Peer, Ilan]

> -----Original Message-----
> From: Jouni Malinen [mailto:j at w1.fi]
> Sent: Monday, February 22, 2016 10:24
> To: Peer, Ilan
> Cc: hostap at lists.infradead.org; Stern, Avraham
> Subject: Re: [PATCH 15/25] WNM: Add candidate list to BSS transition
> response
> 
> On Mon, Feb 15, 2016 at 04:53:41PM +0200, Ilan Peer wrote:
> > Add the transition candidate list to BSS transition response.
> > The candidates preference is set using the regular wpa_supplicant BSS
> > selection logic.
> > If the BSS transition request is rejected and updated scan results are
> > not available, the list is not added.
> 
> > +static int wnm_nei_rep_add_bss(struct wpa_supplicant *wpa_s,
> 
> > +	ie = wpa_bss_get_ie(bss, WLAN_EID_VHT_OPERATION);
> > +	if (ie) {
> > +		vht_oper = (struct ieee80211_vht_operation *)(ie + 2);
> > +
> > +		switch (vht_oper->vht_op_info_chwidth) {
> 
> This could result in buffer overflow since ie[1] was not checked to be large
> enough. I'm just noting this here, but there are similar buffer overflow
> through number of these patches. Whenever using information received
> from an external entity, all the length fields need to be checked to be valid
> before reading data.
> 

Agree. 

> > +		case 2:
> > +			vht = VHT_CHANWIDTH_80MHZ;
> > +			break;
> > +		case 3:
> > +			vht = VHT_CHANWIDTH_160MHZ;
> > +			break;
> > +		case 4:
> > +			vht = VHT_CHANWIDTH_80P80MHZ;
> > +			break;
> > +		default:
> > +			vht = 0;
> > +		}
> > +	}
> > +
> > +	if (ieee80211_freq_to_channel_ext(bss->freq, sec_chan, vht,
> &op_class,
> > +					  &chan) == NUM_HOSTAPD_MODES)
> {
> 
> Why is that conversion from vht_oper->vht_op_info_chwidth to vht used
> here when the Channel Width field in the VHT Operation element is using
> those VHT_CHANWIDTH_* values? The values 2, 3, and 4 here do not match
> the values used in this field and the correct thing to do would seem to be
> simply to set vht = vht_oper->vht_op_info_chwidth.
> 

This is indeed wrong. I think that we got this confused with the values defined in Table 8-151 (HT/VHT Operation Information subfields) as defined in P802.11REVmc D4.3.

Shall I resubmit this patch with the fixes?

Thanks in advance,

Ilan.