Commit 17dff983 authored by Takashi Iwai's avatar Takashi Iwai Committed by Stefan Bader

mwifiex: Fix heap overflow in mwifiex_uap_parse_tail_ies()

A few places in mwifiex_uap_parse_tail_ies() perform memcpy()
unconditionally, which may lead to either buffer overflow or read over
boundary.

This patch addresses the issues by checking the read size and the
destination size at each place more properly.  Along with the fixes,
the patch cleans up the code slightly by introducing a temporary
variable for the token size, and unifies the error path with the
standard goto statement.
Reported-by: default avatarhuangwen <huangwen@venustech.com.cn>
Signed-off-by: default avatarTakashi Iwai <tiwai@suse.de>
Signed-off-by: default avatarKalle Valo <kvalo@codeaurora.org>

CVE-2019-10126

(backported from commit 69ae4f6a)
[tyhicks: Backport to Xenial:
 - There's no need to adjust the WLAN_EID_VENDOR_SPECIFIC case due to
   missing commit bfc83ea1 ("mwifiex: Fix skipped vendor specific
   IEs")
 - Adjust file path due to missing commit 277b024e ("mwifiex: move
   under marvell vendor directory")]
Signed-off-by: default avatarTyler Hicks <tyhicks@canonical.com>
Acked-by: default avatarConnor Kuehl <connor.kuehl@canonical.com>
Acked-by: default avatarKleber Souza <kleber.souza@canonical.com>
Signed-off-by: default avatarKhalid Elmously <khalid.elmously@canonical.com>
parent aa9884ea
...@@ -328,6 +328,8 @@ static int mwifiex_uap_parse_tail_ies(struct mwifiex_private *priv, ...@@ -328,6 +328,8 @@ static int mwifiex_uap_parse_tail_ies(struct mwifiex_private *priv,
struct ieee80211_vendor_ie *vendorhdr; struct ieee80211_vendor_ie *vendorhdr;
u16 gen_idx = MWIFIEX_AUTO_IDX_MASK, ie_len = 0; u16 gen_idx = MWIFIEX_AUTO_IDX_MASK, ie_len = 0;
int left_len, parsed_len = 0; int left_len, parsed_len = 0;
unsigned int token_len;
int err = 0;
if (!info->tail || !info->tail_len) if (!info->tail || !info->tail_len)
return 0; return 0;
...@@ -343,6 +345,12 @@ static int mwifiex_uap_parse_tail_ies(struct mwifiex_private *priv, ...@@ -343,6 +345,12 @@ static int mwifiex_uap_parse_tail_ies(struct mwifiex_private *priv,
*/ */
while (left_len > sizeof(struct ieee_types_header)) { while (left_len > sizeof(struct ieee_types_header)) {
hdr = (void *)(info->tail + parsed_len); hdr = (void *)(info->tail + parsed_len);
token_len = hdr->len + sizeof(struct ieee_types_header);
if (token_len > left_len) {
err = -EINVAL;
goto out;
}
switch (hdr->element_id) { switch (hdr->element_id) {
case WLAN_EID_SSID: case WLAN_EID_SSID:
case WLAN_EID_SUPP_RATES: case WLAN_EID_SUPP_RATES:
...@@ -356,13 +364,16 @@ static int mwifiex_uap_parse_tail_ies(struct mwifiex_private *priv, ...@@ -356,13 +364,16 @@ static int mwifiex_uap_parse_tail_ies(struct mwifiex_private *priv,
case WLAN_EID_VENDOR_SPECIFIC: case WLAN_EID_VENDOR_SPECIFIC:
break; break;
default: default:
memcpy(gen_ie->ie_buffer + ie_len, hdr, if (ie_len + token_len > IEEE_MAX_IE_SIZE) {
hdr->len + sizeof(struct ieee_types_header)); err = -EINVAL;
ie_len += hdr->len + sizeof(struct ieee_types_header); goto out;
}
memcpy(gen_ie->ie_buffer + ie_len, hdr, token_len);
ie_len += token_len;
break; break;
} }
left_len -= hdr->len + sizeof(struct ieee_types_header); left_len -= token_len;
parsed_len += hdr->len + sizeof(struct ieee_types_header); parsed_len += token_len;
} }
/* parse only WPA vendor IE from tail, WMM IE is configured by /* parse only WPA vendor IE from tail, WMM IE is configured by
...@@ -372,15 +383,17 @@ static int mwifiex_uap_parse_tail_ies(struct mwifiex_private *priv, ...@@ -372,15 +383,17 @@ static int mwifiex_uap_parse_tail_ies(struct mwifiex_private *priv,
WLAN_OUI_TYPE_MICROSOFT_WPA, WLAN_OUI_TYPE_MICROSOFT_WPA,
info->tail, info->tail_len); info->tail, info->tail_len);
if (vendorhdr) { if (vendorhdr) {
memcpy(gen_ie->ie_buffer + ie_len, vendorhdr, token_len = vendorhdr->len + sizeof(struct ieee_types_header);
vendorhdr->len + sizeof(struct ieee_types_header)); if (ie_len + token_len > IEEE_MAX_IE_SIZE) {
ie_len += vendorhdr->len + sizeof(struct ieee_types_header); err = -EINVAL;
goto out;
}
memcpy(gen_ie->ie_buffer + ie_len, vendorhdr, token_len);
ie_len += token_len;
} }
if (!ie_len) { if (!ie_len)
kfree(gen_ie); goto out;
return 0;
}
gen_ie->ie_index = cpu_to_le16(gen_idx); gen_ie->ie_index = cpu_to_le16(gen_idx);
gen_ie->mgmt_subtype_mask = cpu_to_le16(MGMT_MASK_BEACON | gen_ie->mgmt_subtype_mask = cpu_to_le16(MGMT_MASK_BEACON |
...@@ -390,13 +403,15 @@ static int mwifiex_uap_parse_tail_ies(struct mwifiex_private *priv, ...@@ -390,13 +403,15 @@ static int mwifiex_uap_parse_tail_ies(struct mwifiex_private *priv,
if (mwifiex_update_uap_custom_ie(priv, gen_ie, &gen_idx, NULL, NULL, if (mwifiex_update_uap_custom_ie(priv, gen_ie, &gen_idx, NULL, NULL,
NULL, NULL)) { NULL, NULL)) {
kfree(gen_ie); err = -EINVAL;
return -1; goto out;
} }
priv->gen_idx = gen_idx; priv->gen_idx = gen_idx;
out:
kfree(gen_ie); kfree(gen_ie);
return 0; return err;
} }
/* This function parses different IEs-head & tail IEs, beacon IEs, /* This function parses different IEs-head & tail IEs, beacon IEs,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment