This was reported by coverity as a use after free. Technically it is correct, though this will not pose a problem since l_queue_remove will not actually dereference the pointer. Reordering this should fix the warning and not change behavior at all. --- src/eapol.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/eapol.c b/src/eapol.c index 6732b370..cbaf0f4f 100644 --- a/src/eapol.c +++ b/src/eapol.c @@ -886,9 +886,9 @@ struct eapol_sm *eapol_sm_new(struct handshake_state *hs) void eapol_sm_free(struct eapol_sm *sm) { - eapol_sm_destroy(sm); - l_queue_remove(state_machines, sm); + + eapol_sm_destroy(sm); } void eapol_sm_set_listen_interval(struct eapol_sm *sm, uint16_t interval) -- 2.17.1

--- src/eapol.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/eapol.c b/src/eapol.c index cbaf0f4f..6f04dbc5 100644 --- a/src/eapol.c +++ b/src/eapol.c @@ -1106,7 +1106,8 @@ static void eapol_handle_ptk_1_of_4(struct eapol_sm *sm, pmkid = handshake_util_find_pmkid_kde(EAPOL_KEY_DATA(ek, sm->mic_len), EAPOL_KEY_DATA_LEN(ek, sm->mic_len)); - ie_parse_rsne_from_data(own_ie, own_ie[1] + 2, &rsn_info); + if (ie_parse_rsne_from_data(own_ie, own_ie[1] + 2, &rsn_info) < 0) + goto error_unspecified; /* * Require the PMKID KDE whenever we've sent a list of PMKIDs in @@ -1557,7 +1558,8 @@ static void eapol_handle_ptk_3_of_4(struct eapol_sm *sm, const uint8_t *mde = sm->handshake->mde; const uint8_t *fte = sm->handshake->fte; - ie_parse_rsne_from_data(rsne, rsne[1] + 2, &ie_info); + if (ie_parse_rsne_from_data(rsne, rsne[1] + 2, &ie_info) < 0) + goto error_ie_different; if (ie_info.num_pmkids != 1 || memcmp(ie_info.pmkids, sm->handshake->pmk_r1_name, 16)) -- 2.17.1

RSNE_ADVANCE could result in a jump to the done label where info would be copied without being initialized. --- src/ie.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ie.c b/src/ie.c index 4bc56589..ba5210c8 100644 --- a/src/ie.c +++ b/src/ie.c @@ -827,13 +827,13 @@ int ie_parse_osen(struct ie_tlv_iter *iter, struct ie_rsn_info *out_info) if (!is_ie_wfa_ie(iter->data, iter->len, IE_WFA_OI_OSEN)) return -EPROTOTYPE; - RSNE_ADVANCE(data, len, 4); - memset(&info, 0, sizeof(info)); info.group_cipher = IE_RSN_CIPHER_SUITE_NO_GROUP_TRAFFIC; info.pairwise_ciphers = IE_RSN_CIPHER_SUITE_CCMP; info.akm_suites = IE_RSN_AKM_SUITE_8021X; + RSNE_ADVANCE(data, len, 4); + if (parse_ciphers(data, len, ie_parse_osen_akm_suite, &info) < 0) return -EBADMSG; -- 2.17.1

Hi James, On 10/16/19 6:43 PM, James Prestwood wrote: Ah but the input is guaranteed to be less than 256 bytes anyway. But in general, using strncpy isn't safe like that either as the target won't be null terminated in some cases. Use l_strlcpy instead. Regards, -Denis