PASN: Fix MIC check not to modify const data

The previous version was using typecasting to ignore const marking for
the input buffer to be able to clear the MIC field for MIC calculation.
That is not really appropriate and could result in issues in the future
if the input data cannot be modified. Fix this by using an allocated
copy of the buffer.

Signed-off-by: Jouni Malinen <j@w1.fi>

diff --git a/src/pasn/pasn_initiator.c b/src/pasn/pasn_initiator.c
index 1c9f23c0f498d07a78d4869ee01e9f5abea6a50c..327516355901c75b48ebd28b5f0387eb067ed110 100644 (file)
--- a/src/pasn/pasn_initiator.c
+++ b/src/pasn/pasn_initiator.c
@@ -1044,6 +1044,8 @@ int wpa_pasn_auth_rx(struct pasn_data *pasn, const u8 *data, size_t len,
        u8 mic_len;
        u16 status;
        int ret, inc_y;
+       u8 *copy = NULL;
+       size_t mic_offset, copy_len;
 
        if (!is_pasn_auth_frame(pasn, mgmt, len))
                return -2;
@@ -1083,12 +1085,8 @@ int wpa_pasn_auth_rx(struct pasn_data *pasn, const u8 *data, size_t len,
                                   "PASN: Invalid MIC. Expecting len=%u",
                                   mic_len);
                        goto fail;
-               } else {
-                       os_memcpy(mic, elems.mic, mic_len);
-                       /* TODO: Clean this up.. Should not be modifying the
-                        * received message buffer. */
-                       os_memset((u8 *) elems.mic, 0, mic_len);
                }
+               os_memcpy(mic, elems.mic, mic_len);
        }
 
        if (!elems.pasn_params || !elems.pasn_params_len) {
@@ -1225,15 +1223,25 @@ int wpa_pasn_auth_rx(struct pasn_data *pasn, const u8 *data, size_t len,
        wpabuf_free(secret);
        secret = NULL;
 
+       /* Use a copy of the message since we need to clear the MIC field */
+       if (!elems.mic)
+               goto fail;
+       mic_offset = elems.mic - (const u8 *) &mgmt->u.auth;
+       copy_len = len - offsetof(struct ieee80211_mgmt, u.auth);
+       if (mic_offset + mic_len > copy_len)
+               goto fail;
+       copy = os_memdup(&mgmt->u.auth, copy_len);
+       if (!copy)
+               goto fail;
+       os_memset(copy + mic_offset, 0, mic_len);
+
        if (pasn->beacon_rsne_rsnxe) {
                /* Verify the MIC */
                ret = pasn_mic(pasn->ptk.kck, pasn->akmp, pasn->cipher,
                               pasn->bssid, pasn->own_addr,
                               wpabuf_head(pasn->beacon_rsne_rsnxe),
                               wpabuf_len(pasn->beacon_rsne_rsnxe),
-                              (u8 *) &mgmt->u.auth,
-                              len - offsetof(struct ieee80211_mgmt, u.auth),
-                              out_mic);
+                              copy, copy_len, out_mic);
        } else {
                u8 *rsne_rsnxe;
                size_t rsne_rsnxe_len = 0;
@@ -1268,12 +1276,12 @@ int wpa_pasn_auth_rx(struct pasn_data *pasn, const u8 *data, size_t len,
                               pasn->bssid, pasn->own_addr,
                               rsne_rsnxe,
                               rsne_rsnxe_len,
-                              (u8 *) &mgmt->u.auth,
-                              len - offsetof(struct ieee80211_mgmt, u.auth),
-                              out_mic);
+                              copy, copy_len, out_mic);
 
                os_free(rsne_rsnxe);
        }
+       os_free(copy);
+       copy = NULL;
 
        wpa_hexdump_key(MSG_DEBUG, "PASN: Frame MIC", mic, mic_len);
        if (ret || os_memcmp(mic, out_mic, mic_len) != 0) {
@@ -1309,6 +1317,7 @@ fail:
        wpa_printf(MSG_DEBUG, "PASN: Failed RX processing - terminating");
        wpabuf_free(wrapped_data);
        wpabuf_free(secret);
+       os_free(copy);
 
        /*
         * TODO: In case of an error the standard allows to silently drop

diff --git a/src/pasn/pasn_responder.c b/src/pasn/pasn_responder.c
index 0c2515cdc05016785c77c05bad329180f4773758..cbc9be852ab6c7c3a6b2c82795c5eea68ba793d2 100644 (file)
--- a/src/pasn/pasn_responder.c
+++ b/src/pasn/pasn_responder.c
@@ -899,6 +899,8 @@ int handle_auth_pasn_3(struct pasn_data *pasn, const u8 *own_addr,
        u8 mic[WPA_PASN_MAX_MIC_LEN], out_mic[WPA_PASN_MAX_MIC_LEN];
        u8 mic_len;
        int ret;
+       u8 *copy = NULL;
+       size_t copy_len, mic_offset;
 
        if (ieee802_11_parse_elems(mgmt->u.auth.variable,
                                   len - offsetof(struct ieee80211_mgmt,
@@ -915,12 +917,8 @@ int handle_auth_pasn_3(struct pasn_data *pasn, const u8 *own_addr,
                wpa_printf(MSG_DEBUG,
                           "PASN: Invalid MIC. Expecting len=%u", mic_len);
                goto fail;
-       } else {
-               os_memcpy(mic, elems.mic, mic_len);
-               /* TODO: Clean this up.. Should not modify received frame
-                * buffer. */
-               os_memset((u8 *) elems.mic, 0, mic_len);
        }
+       os_memcpy(mic, elems.mic, mic_len);
 
        if (!elems.pasn_params || !elems.pasn_params_len) {
                wpa_printf(MSG_DEBUG,
@@ -944,12 +942,21 @@ int handle_auth_pasn_3(struct pasn_data *pasn, const u8 *own_addr,
        }
 
        /* Verify the MIC */
+       copy_len = len - offsetof(struct ieee80211_mgmt, u.auth);
+       mic_offset = elems.mic - (const u8 *) &mgmt->u.auth;
+       copy_len = len - offsetof(struct ieee80211_mgmt, u.auth);
+       if (mic_offset + mic_len > copy_len)
+               goto fail;
+       copy = os_memdup(&mgmt->u.auth, copy_len);
+       if (!copy)
+               goto fail;
+       os_memset(copy + mic_offset, 0, mic_len);
        ret = pasn_mic(pasn->ptk.kck, pasn->akmp, pasn->cipher,
                       peer_addr, own_addr,
                       pasn->hash, mic_len * 2,
-                      (u8 *) &mgmt->u.auth,
-                      len - offsetof(struct ieee80211_mgmt, u.auth),
-                      out_mic);
+                      copy, copy_len, out_mic);
+       os_free(copy);
+       copy = NULL;
 
        wpa_hexdump_key(MSG_DEBUG, "PASN: Frame MIC", mic, mic_len);
        if (ret || os_memcmp(mic, out_mic, mic_len) != 0) {
@@ -996,6 +1003,7 @@ int handle_auth_pasn_3(struct pasn_data *pasn, const u8 *own_addr,
        return 0;
 
 fail:
+       os_free(copy);
        return -1;
 }