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 <michael-dev@fami-braun.de>
This commit is contained in:
Michael Braun 2019-04-28 13:14:57 +02:00 committed by Jouni Malinen
parent d4ceaafc24
commit 963681723f
2 changed files with 19 additions and 49 deletions

View file

@ -2088,6 +2088,9 @@ ieee802_11_set_radius_info(struct hostapd_data *hapd, struct sta_info *sta,
u32 session_timeout = info->session_timeout; u32 session_timeout = info->session_timeout;
u32 acct_interim_interval = info->acct_interim_interval; u32 acct_interim_interval = info->acct_interim_interval;
struct vlan_description *vlan_id = &info->vlan_id; 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 && if (vlan_id->notempty &&
!hostapd_vlan_valid(hapd->conf->vlan, vlan_id)) { !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_LEVEL_INFO, "VLAN ID %d", sta->vlan_id);
hostapd_free_psk_list(sta->psk); hostapd_free_psk_list(sta->psk);
if (hapd->conf->wpa_psk_radius != PSK_RADIUS_IGNORED) { if (hapd->conf->wpa_psk_radius != PSK_RADIUS_IGNORED)
sta->psk = info->psk; hostapd_copy_psk_list(&sta->psk, psk);
info->psk = NULL; else
} else {
sta->psk = NULL; sta->psk = NULL;
}
os_free(sta->identity); os_free(sta->identity);
sta->identity = info->identity; if (identity)
info->identity = NULL; sta->identity = os_strdup(identity);
else
sta->identity = NULL;
os_free(sta->radius_cui); os_free(sta->radius_cui);
sta->radius_cui = info->radius_cui; if (radius_cui)
info->radius_cui = NULL; sta->radius_cui = os_strdup(radius_cui);
else
sta->radius_cui = NULL;
if (hapd->conf->acct_interim_interval == 0 && acct_interim_interval) if (hapd->conf->acct_interim_interval == 0 && acct_interim_interval)
sta->acct_interim_interval = 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; u16 seq_ctrl;
struct radius_sta rad_info; struct radius_sta rad_info;
os_memset(&rad_info, 0, sizeof(rad_info));
if (len < IEEE80211_HDRLEN + sizeof(mgmt->u.auth)) { if (len < IEEE80211_HDRLEN + sizeof(mgmt->u.auth)) {
wpa_printf(MSG_INFO, "handle_auth - too short payload (len=%lu)", wpa_printf(MSG_INFO, "handle_auth - too short payload (len=%lu)",
(unsigned long) len); (unsigned long) len);
@ -2528,10 +2531,6 @@ static void handle_auth(struct hostapd_data *hapd,
} }
fail: 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, reply_res = send_auth_reply(hapd, mgmt->sa, mgmt->bssid, auth_alg,
auth_transaction + 1, resp, resp_ies, auth_transaction + 1, resp, resp_ies,
resp_ies_len, "handle-auth"); resp_ies_len, "handle-auth");
@ -3983,13 +3982,10 @@ static void handle_assoc(struct hostapd_data *hapd,
int left, i; int left, i;
struct sta_info *sta; struct sta_info *sta;
u8 *tmp = NULL; u8 *tmp = NULL;
struct radius_sta info;
#ifdef CONFIG_FILS #ifdef CONFIG_FILS
int delay_assoc = 0; int delay_assoc = 0;
#endif /* CONFIG_FILS */ #endif /* CONFIG_FILS */
os_memset(&info, 0, sizeof(info));
if (len < IEEE80211_HDRLEN + (reassoc ? sizeof(mgmt->u.reassoc_req) : if (len < IEEE80211_HDRLEN + (reassoc ? sizeof(mgmt->u.reassoc_req) :
sizeof(mgmt->u.assoc_req))) { sizeof(mgmt->u.assoc_req))) {
wpa_printf(MSG_INFO, "handle_assoc(reassoc=%d) - too short payload (len=%lu)", 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 == hapd->iface->current_mode->mode ==
HOSTAPD_MODE_IEEE80211AD) { HOSTAPD_MODE_IEEE80211AD) {
int acl_res; int acl_res;
struct radius_sta info;
acl_res = ieee802_11_allowed_address(hapd, mgmt->sa, acl_res = ieee802_11_allowed_address(hapd, mgmt->sa,
(const u8 *) mgmt, (const u8 *) mgmt,
@ -4294,9 +4291,6 @@ static void handle_assoc(struct hostapd_data *hapd,
#endif /* CONFIG_FILS */ #endif /* CONFIG_FILS */
fail: 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. * In case of a successful response, add the station to the driver.

View file

@ -83,24 +83,8 @@ static int hostapd_acl_cache_get(struct hostapd_data *hapd, const u8 *addr,
if (os_reltime_expired(&now, &entry->timestamp, if (os_reltime_expired(&now, &entry->timestamp,
RADIUS_ACL_TIMEOUT)) RADIUS_ACL_TIMEOUT))
return -1; /* entry has expired */ return -1; /* entry has expired */
if (out) { *out = entry->info;
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;
}
return entry->accepted; 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 * @is_probe_req: Whether this query for a Probe Request frame
* Returns: HOSTAPD_ACL_ACCEPT, HOSTAPD_ACL_REJECT, or HOSTAPD_ACL_PENDING * Returns: HOSTAPD_ACL_ACCEPT, HOSTAPD_ACL_REJECT, or HOSTAPD_ACL_PENDING
* *
* The caller is responsible for freeing the returned out.identity and * The caller is responsible for properly cloning the returned out->identity and
* out.radius_cui values with os_free(). * out->radius_cui and out->psk values.
*/ */
int hostapd_allowed_address(struct hostapd_data *hapd, const u8 *addr, int hostapd_allowed_address(struct hostapd_data *hapd, const u8 *addr,
const u8 *msg, size_t len, struct radius_sta *out, 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) { if (os_memcmp(query->addr, addr, ETH_ALEN) == 0) {
/* pending query in RADIUS retransmit queue; /* pending query in RADIUS retransmit queue;
* do not generate a new one */ * 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; return HOSTAPD_ACL_PENDING;
} }
query = query->next; query = query->next;