From be9fbd9b675d8ea1977677e029113514958be6e8 Mon Sep 17 00:00:00 2001 From: Jouni Malinen Date: Fri, 17 Jul 2015 13:00:17 +0300 Subject: [PATCH] FST: Fix FST Action frame length validation Commit 717333f4e4d027c69e7c1241dc124ef7d59a6c85 ('FST: Add the Fast Session Transfer (FST) module') performed incorrect frame length validation for Setup Request (did not remove 24+1 header from consideration) and did not include payload validation for other FST Action frames. Fix these by explicitly verifying that the payload of these frames is sufficiently long before reading the values from there. Signed-off-by: Jouni Malinen --- src/fst/fst_session.c | 45 ++++++++++++++++++++++++++++++++++++------- 1 file changed, 38 insertions(+), 7 deletions(-) diff --git a/src/fst/fst_session.c b/src/fst/fst_session.c index 0f5d72607..ce6bdcc51 100644 --- a/src/fst/fst_session.c +++ b/src/fst/fst_session.c @@ -365,13 +365,16 @@ static void fst_session_handle_setup_request(struct fst_iface *iface, struct fst_group *g; u8 new_iface_peer_addr[ETH_ALEN]; struct wpabuf *peer_mbies; + size_t plen; - if (frame_len < sizeof(*req)) { + if (frame_len < IEEE80211_HDRLEN + 1 + sizeof(*req)) { fst_printf_iface(iface, MSG_WARNING, "FST Request dropped: too short (%zu < %zu)", - frame_len, sizeof(*req)); + frame_len, + IEEE80211_HDRLEN + 1 + sizeof(*req)); return; } + plen = frame_len - IEEE80211_HDRLEN - 1; if (req->stie.new_band_id == req->stie.old_band_id) { fst_printf_iface(iface, MSG_WARNING, @@ -381,9 +384,9 @@ static void fst_session_handle_setup_request(struct fst_iface *iface, g = fst_iface_get_group(iface); - if (frame_len > sizeof(*req)) { + if (plen > sizeof(*req)) { fst_iface_update_mb_ie(iface, mgmt->sa, (const u8 *) (req + 1), - frame_len - sizeof(*req)); + plen - sizeof(*req)); fst_printf_iface(iface, MSG_INFO, "FST Request: MB IEs updated for " MACSTR, MAC2STR(mgmt->sa)); @@ -508,6 +511,7 @@ static void fst_session_handle_setup_response(struct fst_session *s, { const struct fst_setup_res *res = (const struct fst_setup_res *) &mgmt->u.action.u.fst_action; + size_t plen = frame_len - IEEE80211_HDRLEN - 1; enum hostapd_hw_mode hw_mode; u8 channel; union fst_session_state_switch_extra evext = { @@ -528,6 +532,12 @@ static void fst_session_handle_setup_response(struct fst_session *s, return; } + if (plen < sizeof(*res)) { + fst_printf_session(s, MSG_WARNING, + "Too short FST Response dropped"); + return; + } + if (res->dialog_token != s->data.pending_setup_req_dlgt) { fst_printf_session(s, MSG_WARNING, "FST Response dropped due to wrong dialog token (%u != %u)", @@ -596,6 +606,7 @@ static void fst_session_handle_tear_down(struct fst_session *s, { const struct fst_tear_down *td = (const struct fst_tear_down *) &mgmt->u.action.u.fst_action; + size_t plen = frame_len - IEEE80211_HDRLEN - 1; union fst_session_state_switch_extra evext = { .to_initial = { .reason = REASON_TEARDOWN, @@ -608,6 +619,12 @@ static void fst_session_handle_tear_down(struct fst_session *s, return; } + if (plen < sizeof(*td)) { + fst_printf_session(s, MSG_WARNING, + "Too short FST Tear Down dropped"); + return; + } + if (le_to_host32(td->fsts_id) != s->data.fsts_id) { fst_printf_siface(s, iface, MSG_WARNING, "tear down for wrong FST Setup ID (%u)", @@ -628,6 +645,7 @@ static void fst_session_handle_ack_request(struct fst_session *s, { const struct fst_ack_req *req = (const struct fst_ack_req *) &mgmt->u.action.u.fst_action; + size_t plen = frame_len - IEEE80211_HDRLEN - 1; struct fst_ack_res res; union fst_session_state_switch_extra evext = { .to_initial = { @@ -651,6 +669,12 @@ static void fst_session_handle_ack_request(struct fst_session *s, return; } + if (plen < sizeof(*req)) { + fst_printf_session(s, MSG_WARNING, + "Too short FST Ack Request dropped"); + return; + } + if (le_to_host32(req->fsts_id) != s->data.fsts_id) { fst_printf_siface(s, iface, MSG_WARNING, "Ack for wrong FST Setup ID (%u)", @@ -684,6 +708,7 @@ fst_session_handle_ack_response(struct fst_session *s, { const struct fst_ack_res *res = (const struct fst_ack_res *) &mgmt->u.action.u.fst_action; + size_t plen = frame_len - IEEE80211_HDRLEN - 1; union fst_session_state_switch_extra evext = { .to_initial = { .reason = REASON_SWITCH, @@ -706,6 +731,12 @@ fst_session_handle_ack_response(struct fst_session *s, return; } + if (plen < sizeof(*res)) { + fst_printf_session(s, MSG_WARNING, + "Too short FST Ack Response dropped"); + return; + } + if (le_to_host32(res->fsts_id) != s->data.fsts_id) { fst_printf_siface(s, iface, MSG_ERROR, "Ack response for wrong FST Setup ID (%u)", @@ -1164,10 +1195,10 @@ void fst_session_on_action_rx(struct fst_iface *iface, { struct fst_session *s; - if (mgmt->u.action.category != WLAN_ACTION_FST) { + if (len < IEEE80211_HDRLEN + 2 || + mgmt->u.action.category != WLAN_ACTION_FST) { fst_printf_iface(iface, MSG_ERROR, - "action frame of wrong category (%u) received!", - mgmt->u.action.category); + "invalid Action frame received"); return; }