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>

diff --git a/src/ap/ieee802_11.c b/src/ap/ieee802_11.c
index 6859ddecf286186873a2885b387dbd278feb5d5f..ef1078ff0398f424ea7a9b9037a45751bf40cfa0 100644 (file)
--- 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 6295b996d4b1c5c1c6d94b55f3b235060cc93d41..783ee6dea96b22357fd532d03584cff99d705e15 100644 (file)
--- 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;