From 963681723f3b0a2eafe5c3f8fa1defbc112f671e Mon Sep 17 00:00:00 2001 From: Michael Braun Date: Sun, 28 Apr 2019 13:14:57 +0200 Subject: [PATCH] Fix possible memory leak of RADIUS data in handle_auth() When returning from handle_auth() after ieee802_11_allowed_address() returned HOSTAPD_ACL_ACCEPT, but before ieee802_11_set_radius_info() has been called, identity, radius_cui, and psk might not have been consumed. Fix this by avoiding the need to free these variables at all. Signed-off-by: Michael Braun --- src/ap/ieee802_11.c | 36 +++++++++++++++--------------------- src/ap/ieee802_11_auth.c | 32 ++++---------------------------- 2 files changed, 19 insertions(+), 49 deletions(-) diff --git a/src/ap/ieee802_11.c b/src/ap/ieee802_11.c index 6859ddecf..ef1078ff0 100644 --- a/src/ap/ieee802_11.c +++ b/src/ap/ieee802_11.c @@ -2088,6 +2088,9 @@ ieee802_11_set_radius_info(struct hostapd_data *hapd, struct sta_info *sta, u32 session_timeout = info->session_timeout; u32 acct_interim_interval = info->acct_interim_interval; struct vlan_description *vlan_id = &info->vlan_id; + struct hostapd_sta_wpa_psk_short *psk = info->psk; + char *identity = info->identity; + char *radius_cui = info->radius_cui; if (vlan_id->notempty && !hostapd_vlan_valid(hapd->conf->vlan, vlan_id)) { @@ -2105,20 +2108,22 @@ ieee802_11_set_radius_info(struct hostapd_data *hapd, struct sta_info *sta, HOSTAPD_LEVEL_INFO, "VLAN ID %d", sta->vlan_id); hostapd_free_psk_list(sta->psk); - if (hapd->conf->wpa_psk_radius != PSK_RADIUS_IGNORED) { - sta->psk = info->psk; - info->psk = NULL; - } else { + if (hapd->conf->wpa_psk_radius != PSK_RADIUS_IGNORED) + hostapd_copy_psk_list(&sta->psk, psk); + else sta->psk = NULL; - } os_free(sta->identity); - sta->identity = info->identity; - info->identity = NULL; + if (identity) + sta->identity = os_strdup(identity); + else + sta->identity = NULL; os_free(sta->radius_cui); - sta->radius_cui = info->radius_cui; - info->radius_cui = NULL; + if (radius_cui) + sta->radius_cui = os_strdup(radius_cui); + else + sta->radius_cui = NULL; if (hapd->conf->acct_interim_interval == 0 && acct_interim_interval) sta->acct_interim_interval = acct_interim_interval; @@ -2151,8 +2156,6 @@ static void handle_auth(struct hostapd_data *hapd, u16 seq_ctrl; struct radius_sta rad_info; - os_memset(&rad_info, 0, sizeof(rad_info)); - if (len < IEEE80211_HDRLEN + sizeof(mgmt->u.auth)) { wpa_printf(MSG_INFO, "handle_auth - too short payload (len=%lu)", (unsigned long) len); @@ -2528,10 +2531,6 @@ static void handle_auth(struct hostapd_data *hapd, } fail: - os_free(rad_info.identity); - os_free(rad_info.radius_cui); - hostapd_free_psk_list(rad_info.psk); - reply_res = send_auth_reply(hapd, mgmt->sa, mgmt->bssid, auth_alg, auth_transaction + 1, resp, resp_ies, resp_ies_len, "handle-auth"); @@ -3983,13 +3982,10 @@ static void handle_assoc(struct hostapd_data *hapd, int left, i; struct sta_info *sta; u8 *tmp = NULL; - struct radius_sta info; #ifdef CONFIG_FILS int delay_assoc = 0; #endif /* CONFIG_FILS */ - os_memset(&info, 0, sizeof(info)); - if (len < IEEE80211_HDRLEN + (reassoc ? sizeof(mgmt->u.reassoc_req) : sizeof(mgmt->u.assoc_req))) { wpa_printf(MSG_INFO, "handle_assoc(reassoc=%d) - too short payload (len=%lu)", @@ -4065,6 +4061,7 @@ static void handle_assoc(struct hostapd_data *hapd, hapd->iface->current_mode->mode == HOSTAPD_MODE_IEEE80211AD) { int acl_res; + struct radius_sta info; acl_res = ieee802_11_allowed_address(hapd, mgmt->sa, (const u8 *) mgmt, @@ -4294,9 +4291,6 @@ static void handle_assoc(struct hostapd_data *hapd, #endif /* CONFIG_FILS */ fail: - os_free(info.identity); - os_free(info.radius_cui); - hostapd_free_psk_list(info.psk); /* * In case of a successful response, add the station to the driver. diff --git a/src/ap/ieee802_11_auth.c b/src/ap/ieee802_11_auth.c index 6295b996d..783ee6dea 100644 --- a/src/ap/ieee802_11_auth.c +++ b/src/ap/ieee802_11_auth.c @@ -83,24 +83,8 @@ static int hostapd_acl_cache_get(struct hostapd_data *hapd, const u8 *addr, if (os_reltime_expired(&now, &entry->timestamp, RADIUS_ACL_TIMEOUT)) return -1; /* entry has expired */ - if (out) { - if (entry->accepted == HOSTAPD_ACL_ACCEPT_TIMEOUT) - out->session_timeout = - entry->info.session_timeout; - out->acct_interim_interval = - entry->info.acct_interim_interval; - out->vlan_id = entry->info.vlan_id; - copy_psk_list(&out->psk, entry->info.psk); - if (entry->info.identity) - out->identity = os_strdup(entry->info.identity); - else - out->identity = NULL; - if (entry->info.radius_cui) - out->radius_cui = - os_strdup(entry->info.radius_cui); - else - out->radius_cui = NULL; - } + *out = entry->info; + return entry->accepted; } @@ -223,8 +207,8 @@ int hostapd_check_acl(struct hostapd_data *hapd, const u8 *addr, * @is_probe_req: Whether this query for a Probe Request frame * Returns: HOSTAPD_ACL_ACCEPT, HOSTAPD_ACL_REJECT, or HOSTAPD_ACL_PENDING * - * The caller is responsible for freeing the returned out.identity and - * out.radius_cui values with os_free(). + * The caller is responsible for properly cloning the returned out->identity and + * out->radius_cui and out->psk values. */ int hostapd_allowed_address(struct hostapd_data *hapd, const u8 *addr, const u8 *msg, size_t len, struct radius_sta *out, @@ -266,14 +250,6 @@ int hostapd_allowed_address(struct hostapd_data *hapd, const u8 *addr, if (os_memcmp(query->addr, addr, ETH_ALEN) == 0) { /* pending query in RADIUS retransmit queue; * do not generate a new one */ - if (out && out->identity) { - os_free(out->identity); - out->identity = NULL; - } - if (out && out->radius_cui) { - os_free(out->radius_cui); - out->radius_cui = NULL; - } return HOSTAPD_ACL_PENDING; } query = query->next; -- 2.39.5