Browse Source

GAS server: Fix request frame length validation (CID 68098)

There seemed to be an off-by-one error in the validation of GAS request
frames. If a Public Action frame without the Action code field would
have reached this function, the length could have been passed as
(size_t) -1 which would likely have resulted in a crash due to reading
beyond the buffer. However, it looks like such frame would not be
delivered to hostapd at least with mac80211-based drivers. Anyway, this
function better be more careful with length validation should some other
driver end up reporting invalid Action frames.

In addition, the Action code field is in a fixed location, so the
IEEE80211_HDRLEN can be used here to clean up bounds checking to avoid
false reports from static analyzer.

Signed-off-by: Jouni Malinen <j@w1.fi>
Jouni Malinen 10 years ago
parent
commit
062833c67c
1 changed files with 3 additions and 5 deletions
  1. 3 5
      src/ap/gas_serv.c

+ 3 - 5
src/ap/gas_serv.c

@@ -1213,13 +1213,11 @@ static void gas_serv_rx_public_action(void *ctx, const u8 *buf, size_t len,
 {
 	struct hostapd_data *hapd = ctx;
 	const struct ieee80211_mgmt *mgmt;
-	size_t hdr_len;
 	const u8 *sa, *data;
 	int prot;
 
 	mgmt = (const struct ieee80211_mgmt *) buf;
-	hdr_len = (const u8 *) &mgmt->u.action.u.vs_public_action.action - buf;
-	if (hdr_len > len)
+	if (len < IEEE80211_HDRLEN + 2)
 		return;
 	if (mgmt->u.action.category != WLAN_ACTION_PUBLIC &&
 	    mgmt->u.action.category != WLAN_ACTION_PROTECTED_DUAL)
@@ -1231,8 +1229,8 @@ static void gas_serv_rx_public_action(void *ctx, const u8 *buf, size_t len,
 	 */
 	prot = mgmt->u.action.category == WLAN_ACTION_PROTECTED_DUAL;
 	sa = mgmt->sa;
-	len -= hdr_len;
-	data = &mgmt->u.action.u.public_action.action;
+	len -= IEEE80211_HDRLEN + 1;
+	data = buf + IEEE80211_HDRLEN + 1;
 	switch (data[0]) {
 	case WLAN_PA_GAS_INITIAL_REQ:
 		gas_serv_rx_gas_initial_req(hapd, sa, data + 1, len - 1, prot);