From d6ee858c3b11503d3ed2600f7380719cb2bb52dd Mon Sep 17 00:00:00 2001 From: Jouni Malinen Date: Sun, 18 Oct 2015 01:45:22 +0300 Subject: [PATCH] P2P: Avoid undefined behavior in pointer arithmetic Reorder terms in a way that no invalid pointers are generated with pos+len operations. end-pos is always defined (with a valid pos pointer) while pos+len could end up pointing beyond the end pointer which would be undefined behavior. Signed-off-by: Jouni Malinen --- src/p2p/p2p.c | 4 +-- src/p2p/p2p_go_neg.c | 8 +++--- src/p2p/p2p_group.c | 4 +-- src/p2p/p2p_parse.c | 67 ++++++++++++++++++++++---------------------- src/p2p/p2p_sd.c | 46 +++++++++++++++--------------- 5 files changed, 64 insertions(+), 65 deletions(-) diff --git a/src/p2p/p2p.c b/src/p2p/p2p.c index 071a69629..3e26011a0 100644 --- a/src/p2p/p2p.c +++ b/src/p2p/p2p.c @@ -636,11 +636,11 @@ static void p2p_update_peer_vendor_elems(struct p2p_device *dev, const u8 *ies, end = ies + ies_len; - for (pos = ies; pos + 1 < end; pos += len) { + for (pos = ies; end - pos > 1; pos += len) { id = *pos++; len = *pos++; - if (pos + len > end) + if (len > end - pos) break; if (id != WLAN_EID_VENDOR_SPECIFIC || len < 3) diff --git a/src/p2p/p2p_go_neg.c b/src/p2p/p2p_go_neg.c index dfbf14ad7..096ccd60a 100644 --- a/src/p2p/p2p_go_neg.c +++ b/src/p2p/p2p_go_neg.c @@ -38,7 +38,7 @@ int p2p_peer_channels_check(struct p2p_data *p2p, struct p2p_channels *own, { const u8 *pos, *end; struct p2p_channels *ch; - size_t channels; + u8 channels; struct p2p_channels intersection; ch = &dev->channels; @@ -58,14 +58,14 @@ int p2p_peer_channels_check(struct p2p_data *p2p, struct p2p_channels *own, } pos += 3; - while (pos + 2 < end) { + while (end - pos > 2) { struct p2p_reg_class *cl = &ch->reg_class[ch->reg_classes]; cl->reg_class = *pos++; - if (pos + 1 + pos[0] > end) { + channels = *pos++; + if (channels > end - pos) { p2p_info(p2p, "Invalid peer Channel List"); return -1; } - channels = *pos++; cl->channels = channels > P2P_MAX_REG_CLASS_CHANNELS ? P2P_MAX_REG_CLASS_CHANNELS : channels; os_memcpy(cl->channel, pos, cl->channels); diff --git a/src/p2p/p2p_group.c b/src/p2p/p2p_group.c index 0d6699346..2cf245077 100644 --- a/src/p2p/p2p_group.c +++ b/src/p2p/p2p_group.c @@ -296,14 +296,14 @@ static int wifi_display_add_dev_info_descr(struct wpabuf *buf, os_memset(zero_addr, 0, ETH_ALEN); pos = wpabuf_head_u8(m->wfd_ie); end = pos + wpabuf_len(m->wfd_ie); - while (pos + 1 < end) { + while (end - pos >= 3) { u8 id; u16 len; id = *pos++; len = WPA_GET_BE16(pos); pos += 2; - if (pos + len > end) + if (len > end - pos) break; switch (id) { diff --git a/src/p2p/p2p_parse.c b/src/p2p/p2p_parse.c index bd1e68bd4..9e16fc62f 100644 --- a/src/p2p/p2p_parse.c +++ b/src/p2p/p2p_parse.c @@ -19,7 +19,8 @@ static int p2p_parse_attribute(u8 id, const u8 *data, u16 len, struct p2p_message *msg) { const u8 *pos; - size_t i, nlen; + size_t i; + u16 nlen; char devtype[WPS_DEV_TYPE_BUFSIZE]; switch (id) { @@ -149,10 +150,9 @@ static int p2p_parse_attribute(u8 id, const u8 *data, u16 len, pos += 2; nlen = WPA_GET_BE16(pos); pos += 2; - if (data + len - pos < (int) nlen || - nlen > WPS_DEV_NAME_MAX_LEN) { + if (nlen > data + len - pos || nlen > WPS_DEV_NAME_MAX_LEN) { wpa_printf(MSG_DEBUG, "P2P: Invalid Device Name " - "length %d (buf len %d)", (int) nlen, + "length %u (buf len %d)", nlen, (int) (data + len - pos)); return -1; } @@ -637,49 +637,48 @@ int p2p_group_info_parse(const u8 *gi, size_t gi_len, gend = gi + gi_len; while (g < gend) { struct p2p_client_info *cli; - const u8 *t, *cend; - int count; + const u8 *cend; + u16 count; + u8 len; cli = &info->client[info->num_clients]; - cend = g + 1 + g[0]; - if (cend > gend) + len = *g++; + if (len > gend - g || len < 2 * ETH_ALEN + 1 + 2 + 8 + 1) return -1; /* invalid data */ + cend = g + len; /* g at start of P2P Client Info Descriptor */ - /* t at Device Capability Bitmap */ - t = g + 1 + 2 * ETH_ALEN; - if (t > cend) - return -1; /* invalid data */ - cli->p2p_device_addr = g + 1; - cli->p2p_interface_addr = g + 1 + ETH_ALEN; - cli->dev_capab = t[0]; + cli->p2p_device_addr = g; + g += ETH_ALEN; + cli->p2p_interface_addr = g; + g += ETH_ALEN; + cli->dev_capab = *g++; - if (t + 1 + 2 + 8 + 1 > cend) - return -1; /* invalid data */ + cli->config_methods = WPA_GET_BE16(g); + g += 2; + cli->pri_dev_type = g; + g += 8; - cli->config_methods = WPA_GET_BE16(&t[1]); - cli->pri_dev_type = &t[3]; - - t += 1 + 2 + 8; - /* t at Number of Secondary Device Types */ - cli->num_sec_dev_types = *t++; - if (t + 8 * cli->num_sec_dev_types > cend) + /* g at Number of Secondary Device Types */ + len = *g++; + if (8 * len > cend - g) return -1; /* invalid data */ - cli->sec_dev_types = t; - t += 8 * cli->num_sec_dev_types; + cli->num_sec_dev_types = len; + cli->sec_dev_types = g; + g += 8 * len; - /* t at Device Name in WPS TLV format */ - if (t + 2 + 2 > cend) + /* g at Device Name in WPS TLV format */ + if (cend - g < 2 + 2) return -1; /* invalid data */ - if (WPA_GET_BE16(t) != ATTR_DEV_NAME) + if (WPA_GET_BE16(g) != ATTR_DEV_NAME) return -1; /* invalid Device Name TLV */ - t += 2; - count = WPA_GET_BE16(t); - t += 2; - if (count > cend - t) + g += 2; + count = WPA_GET_BE16(g); + g += 2; + if (count > cend - g) return -1; /* invalid Device Name TLV */ if (count >= WPS_DEV_NAME_MAX_LEN) count = WPS_DEV_NAME_MAX_LEN; - cli->dev_name = (const char *) t; + cli->dev_name = (const char *) g; cli->dev_name_len = count; g = cend; diff --git a/src/p2p/p2p_sd.c b/src/p2p/p2p_sd.c index 1a2af04b8..559af9d98 100644 --- a/src/p2p/p2p_sd.c +++ b/src/p2p/p2p_sd.c @@ -28,11 +28,11 @@ static int wfd_wsd_supported(struct wpabuf *wfd) pos = wpabuf_head(wfd); end = pos + wpabuf_len(wfd); - while (pos + 3 <= end) { + while (end - pos >= 3) { subelem = *pos++; len = WPA_GET_BE16(pos); pos += 2; - if (pos + len > end) + if (len > end - pos) break; if (subelem == WFD_SUBELEM_DEVICE_INFO && len >= 6) { @@ -355,11 +355,11 @@ void p2p_rx_gas_initial_req(struct p2p_data *p2p, const u8 *sa, pos++; slen = *pos++; - next = pos + slen; - if (next > end || slen < 2) { + if (slen > end - pos || slen < 2) { p2p_dbg(p2p, "Invalid IE in GAS Initial Request"); return; } + next = pos + slen; pos++; /* skip QueryRespLenLimit and PAME-BI */ if (*pos != ACCESS_NETWORK_QUERY_PROTOCOL) { @@ -370,16 +370,16 @@ void p2p_rx_gas_initial_req(struct p2p_data *p2p, const u8 *sa, pos = next; /* Query Request */ - if (pos + 2 > end) + if (end - pos < 2) return; slen = WPA_GET_LE16(pos); pos += 2; - if (pos + slen > end) + if (slen > end - pos) return; end = pos + slen; /* ANQP Query Request */ - if (pos + 4 > end) + if (end - pos < 4) return; if (WPA_GET_LE16(pos) != ANQP_VENDOR_SPECIFIC) { p2p_dbg(p2p, "Unsupported ANQP Info ID %u", WPA_GET_LE16(pos)); @@ -389,7 +389,7 @@ void p2p_rx_gas_initial_req(struct p2p_data *p2p, const u8 *sa, slen = WPA_GET_LE16(pos); pos += 2; - if (pos + slen > end || slen < 3 + 1) { + if (slen > end - pos || slen < 3 + 1) { p2p_dbg(p2p, "Invalid ANQP Query Request length"); return; } @@ -401,7 +401,7 @@ void p2p_rx_gas_initial_req(struct p2p_data *p2p, const u8 *sa, } pos += 4; - if (pos + 2 > end) + if (end - pos < 2) return; update_indic = WPA_GET_LE16(pos); p2p_dbg(p2p, "Service Update Indicator: %u", update_indic); @@ -512,11 +512,11 @@ void p2p_rx_gas_initial_resp(struct p2p_data *p2p, const u8 *sa, pos++; slen = *pos++; - next = pos + slen; - if (next > end || slen < 2) { + if (slen > end - pos || slen < 2) { p2p_dbg(p2p, "Invalid IE in GAS Initial Response"); return; } + next = pos + slen; pos++; /* skip QueryRespLenLimit and PAME-BI */ if (*pos != ACCESS_NETWORK_QUERY_PROTOCOL) { @@ -527,14 +527,14 @@ void p2p_rx_gas_initial_resp(struct p2p_data *p2p, const u8 *sa, pos = next; /* Query Response */ - if (pos + 2 > end) { + if (end - pos < 2) { p2p_dbg(p2p, "Too short Query Response"); return; } slen = WPA_GET_LE16(pos); pos += 2; p2p_dbg(p2p, "Query Response Length: %d", slen); - if (pos + slen > end) { + if (slen > end - pos) { p2p_dbg(p2p, "Not enough Query Response data"); return; } @@ -552,7 +552,7 @@ void p2p_rx_gas_initial_resp(struct p2p_data *p2p, const u8 *sa, } /* ANQP Query Response */ - if (pos + 4 > end) + if (end - pos < 4) return; if (WPA_GET_LE16(pos) != ANQP_VENDOR_SPECIFIC) { p2p_dbg(p2p, "Unsupported ANQP Info ID %u", WPA_GET_LE16(pos)); @@ -562,7 +562,7 @@ void p2p_rx_gas_initial_resp(struct p2p_data *p2p, const u8 *sa, slen = WPA_GET_LE16(pos); pos += 2; - if (pos + slen > end || slen < 3 + 1) { + if (slen > end - pos || slen < 3 + 1) { p2p_dbg(p2p, "Invalid ANQP Query Response length"); return; } @@ -574,7 +574,7 @@ void p2p_rx_gas_initial_resp(struct p2p_data *p2p, const u8 *sa, } pos += 4; - if (pos + 2 > end) + if (end - pos < 2) return; update_indic = WPA_GET_LE16(pos); p2p_dbg(p2p, "Service Update Indicator: %u", update_indic); @@ -727,11 +727,11 @@ void p2p_rx_gas_comeback_resp(struct p2p_data *p2p, const u8 *sa, pos++; slen = *pos++; - next = pos + slen; - if (next > end || slen < 2) { + if (slen > end - pos || slen < 2) { p2p_dbg(p2p, "Invalid IE in GAS Comeback Response"); return; } + next = pos + slen; pos++; /* skip QueryRespLenLimit and PAME-BI */ if (*pos != ACCESS_NETWORK_QUERY_PROTOCOL) { @@ -742,14 +742,14 @@ void p2p_rx_gas_comeback_resp(struct p2p_data *p2p, const u8 *sa, pos = next; /* Query Response */ - if (pos + 2 > end) { + if (end - pos < 2) { p2p_dbg(p2p, "Too short Query Response"); return; } slen = WPA_GET_LE16(pos); pos += 2; p2p_dbg(p2p, "Query Response Length: %d", slen); - if (pos + slen > end) { + if (slen > end - pos) { p2p_dbg(p2p, "Not enough Query Response data"); return; } @@ -768,7 +768,7 @@ void p2p_rx_gas_comeback_resp(struct p2p_data *p2p, const u8 *sa, } /* ANQP Query Response */ - if (pos + 4 > end) + if (end - pos < 4) return; if (WPA_GET_LE16(pos) != ANQP_VENDOR_SPECIFIC) { p2p_dbg(p2p, "Unsupported ANQP Info ID %u", WPA_GET_LE16(pos)); @@ -783,7 +783,7 @@ void p2p_rx_gas_comeback_resp(struct p2p_data *p2p, const u8 *sa, p2p_dbg(p2p, "Invalid ANQP Query Response length"); return; } - if (pos + 4 > end) + if (end - pos < 4) return; if (WPA_GET_BE32(pos) != P2P_IE_VENDOR_TYPE) { @@ -793,7 +793,7 @@ void p2p_rx_gas_comeback_resp(struct p2p_data *p2p, const u8 *sa, } pos += 4; - if (pos + 2 > end) + if (end - pos < 2) return; p2p->sd_rx_update_indic = WPA_GET_LE16(pos); p2p_dbg(p2p, "Service Update Indicator: %u", p2p->sd_rx_update_indic);