Add explicit buffer length checks for p2p_build_wps_ie()

Even though the length of this buffer is based only on locally
configured information, it is cleaner to include explicit buffer room
validation steps when adding the attributes into the buffer.

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

src/p2p/p2p.c

@@ -1911,7 +1911,11 @@ struct wpabuf * p2p_build_probe_resp_ies(struct p2p_data *p2p)
 		pw_id = p2p_wps_method_pw_id(p2p->go_neg_peer->wps_method);
 	}
 
-	p2p_build_wps_ie(p2p, buf, pw_id, 1);
+	if (p2p_build_wps_ie(p2p, buf, pw_id, 1) < 0) {
+		p2p_dbg(p2p, "Failed to build WPS IE for Probe Response");
+		wpabuf_free(buf);
+		return NULL;
+	}
 
 #ifdef CONFIG_WIFI_DISPLAY
 	if (p2p->wfd_ie_probe_resp)

src/p2p/p2p_build.c

@@ -327,13 +327,15 @@ void p2p_buf_add_p2p_interface(struct wpabuf *buf, struct p2p_data *p2p)
 }
 
 
-static void p2p_add_wps_string(struct wpabuf *buf, enum wps_attribute attr,
-			       const char *val)
+static int p2p_add_wps_string(struct wpabuf *buf, enum wps_attribute attr,
+			      const char *val)
 {
 	size_t len;
 
-	wpabuf_put_be16(buf, attr);
 	len = val ? os_strlen(val) : 0;
+	if (wpabuf_tailroom(buf) < 4 + len)
+		return -1;
+	wpabuf_put_be16(buf, attr);
 #ifndef CONFIG_WPS_STRICT
 	if (len == 0) {
 		/*
@@ -341,36 +343,46 @@ static void p2p_add_wps_string(struct wpabuf *buf, enum wps_attribute attr,
 		 * attributes. As a workaround, send a space character if the
 		 * device attribute string is empty.
 		 */
+		if (wpabuf_tailroom(buf) < 3)
+			return -1;
 		wpabuf_put_be16(buf, 1);
 		wpabuf_put_u8(buf, ' ');
-		return;
+		return 0;
 	}
 #endif /* CONFIG_WPS_STRICT */
 	wpabuf_put_be16(buf, len);
 	if (val)
 		wpabuf_put_data(buf, val, len);
+	return 0;
 }
 
 
-void p2p_build_wps_ie(struct p2p_data *p2p, struct wpabuf *buf, int pw_id,
-		      int all_attr)
+int p2p_build_wps_ie(struct p2p_data *p2p, struct wpabuf *buf, int pw_id,
+		     int all_attr)
 {
 	u8 *len;
 	int i;
 
+	if (wpabuf_tailroom(buf) < 6)
+		return -1;
 	wpabuf_put_u8(buf, WLAN_EID_VENDOR_SPECIFIC);
 	len = wpabuf_put(buf, 1);
 	wpabuf_put_be32(buf, WPS_DEV_OUI_WFA);
 
-	wps_build_version(buf);
+	if (wps_build_version(buf) < 0)
+		return -1;
 
 	if (all_attr) {
+		if (wpabuf_tailroom(buf) < 5)
+			return -1;
 		wpabuf_put_be16(buf, ATTR_WPS_STATE);
 		wpabuf_put_be16(buf, 1);
 		wpabuf_put_u8(buf, WPS_STATE_NOT_CONFIGURED);
 	}
 
 	if (pw_id >= 0) {
+		if (wpabuf_tailroom(buf) < 6)
+			return -1;
 		/* Device Password ID */
 		wpabuf_put_be16(buf, ATTR_DEV_PASSWORD_ID);
 		wpabuf_put_be16(buf, 2);
@@ -380,33 +392,47 @@ void p2p_build_wps_ie(struct p2p_data *p2p, struct wpabuf *buf, int pw_id,
 	}
 
 	if (all_attr) {
+		if (wpabuf_tailroom(buf) < 5)
+			return -1;
 		wpabuf_put_be16(buf, ATTR_RESPONSE_TYPE);
 		wpabuf_put_be16(buf, 1);
 		wpabuf_put_u8(buf, WPS_RESP_ENROLLEE_INFO);
 
-		wps_build_uuid_e(buf, p2p->cfg->uuid);
-		p2p_add_wps_string(buf, ATTR_MANUFACTURER,
-				   p2p->cfg->manufacturer);
-		p2p_add_wps_string(buf, ATTR_MODEL_NAME, p2p->cfg->model_name);
-		p2p_add_wps_string(buf, ATTR_MODEL_NUMBER,
-				   p2p->cfg->model_number);
-		p2p_add_wps_string(buf, ATTR_SERIAL_NUMBER,
-				   p2p->cfg->serial_number);
-
+		if (wps_build_uuid_e(buf, p2p->cfg->uuid) < 0 ||
+		    p2p_add_wps_string(buf, ATTR_MANUFACTURER,
+				       p2p->cfg->manufacturer) < 0 ||
+		    p2p_add_wps_string(buf, ATTR_MODEL_NAME,
+				       p2p->cfg->model_name) < 0 ||
+		    p2p_add_wps_string(buf, ATTR_MODEL_NUMBER,
+				       p2p->cfg->model_number) < 0 ||
+		    p2p_add_wps_string(buf, ATTR_SERIAL_NUMBER,
+				       p2p->cfg->serial_number) < 0)
+			return -1;
+
+		if (wpabuf_tailroom(buf) < 4 + WPS_DEV_TYPE_LEN)
+			return -1;
 		wpabuf_put_be16(buf, ATTR_PRIMARY_DEV_TYPE);
 		wpabuf_put_be16(buf, WPS_DEV_TYPE_LEN);
 		wpabuf_put_data(buf, p2p->cfg->pri_dev_type, WPS_DEV_TYPE_LEN);
 
-		p2p_add_wps_string(buf, ATTR_DEV_NAME, p2p->cfg->dev_name);
+		if (p2p_add_wps_string(buf, ATTR_DEV_NAME, p2p->cfg->dev_name)
+		    < 0)
+			return -1;
 
+		if (wpabuf_tailroom(buf) < 6)
+			return -1;
 		wpabuf_put_be16(buf, ATTR_CONFIG_METHODS);
 		wpabuf_put_be16(buf, 2);
 		wpabuf_put_be16(buf, p2p->cfg->config_methods);
 	}
 
-	wps_build_wfa_ext(buf, 0, NULL, 0);
+	if (wps_build_wfa_ext(buf, 0, NULL, 0) < 0)
+		return -1;
 
 	if (all_attr && p2p->cfg->num_sec_dev_types) {
+		if (wpabuf_tailroom(buf) <
+		    4 + WPS_DEV_TYPE_LEN * p2p->cfg->num_sec_dev_types)
+			return -1;
 		wpabuf_put_be16(buf, ATTR_SECONDARY_DEV_TYPE_LIST);
 		wpabuf_put_be16(buf, WPS_DEV_TYPE_LEN *
 				p2p->cfg->num_sec_dev_types);
@@ -428,4 +454,6 @@ void p2p_build_wps_ie(struct p2p_data *p2p, struct wpabuf *buf, int pw_id,
 	}
 
 	p2p_buf_update_ie_hdr(buf, len);
+
+	return 0;
 }

src/p2p/p2p_go_neg.c

@@ -172,7 +172,12 @@ static struct wpabuf * p2p_build_go_neg_req(struct p2p_data *p2p,
 	p2p_buf_update_ie_hdr(buf, len);
 
 	/* WPS IE with Device Password ID attribute */
-	p2p_build_wps_ie(p2p, buf, p2p_wps_method_pw_id(peer->wps_method), 0);
+	if (p2p_build_wps_ie(p2p, buf, p2p_wps_method_pw_id(peer->wps_method),
+			     0) < 0) {
+		p2p_dbg(p2p, "Failed to build WPS IE for GO Negotiation Request");
+		wpabuf_free(buf);
+		return NULL;
+	}
 
 #ifdef CONFIG_WIFI_DISPLAY
 	if (p2p->wfd_ie_go_neg)
@@ -307,9 +312,13 @@ static struct wpabuf * p2p_build_go_neg_resp(struct p2p_data *p2p,
 	p2p_buf_update_ie_hdr(buf, len);
 
 	/* WPS IE with Device Password ID attribute */
-	p2p_build_wps_ie(p2p, buf,
-			 p2p_wps_method_pw_id(peer ? peer->wps_method :
-					      WPS_NOT_READY), 0);
+	if (p2p_build_wps_ie(p2p, buf,
+			     p2p_wps_method_pw_id(peer ? peer->wps_method :
+						  WPS_NOT_READY), 0) < 0) {
+		p2p_dbg(p2p, "Failed to build WPS IE for GO Negotiation Response");
+		wpabuf_free(buf);
+		return NULL;
+	}
 
 #ifdef CONFIG_WIFI_DISPLAY
 	if (p2p->wfd_ie_go_neg)

src/p2p/p2p_i.h

@@ -633,8 +633,8 @@ void p2p_buf_add_noa(struct wpabuf *buf, u8 noa_index, u8 opp_ps, u8 ctwindow,
 void p2p_buf_add_ext_listen_timing(struct wpabuf *buf, u16 period,
 				   u16 interval);
 void p2p_buf_add_p2p_interface(struct wpabuf *buf, struct p2p_data *p2p);
-void p2p_build_wps_ie(struct p2p_data *p2p, struct wpabuf *buf, int pw_id,
-		      int all_attr);
+int p2p_build_wps_ie(struct p2p_data *p2p, struct wpabuf *buf, int pw_id,
+		     int all_attr);
 
 /* p2p_sd.c */
 struct p2p_sd_query * p2p_pending_sd_req(struct p2p_data *p2p,

src/wps/wps_attr_build.c

@@ -118,6 +118,8 @@ int wps_build_config_methods(struct wpabuf *msg, u16 methods)
 
 int wps_build_uuid_e(struct wpabuf *msg, const u8 *uuid)
 {
+	if (wpabuf_tailroom(msg) < 4 + WPS_UUID_LEN)
+		return -1;
 	wpa_printf(MSG_DEBUG, "WPS:  * UUID-E");
 	wpabuf_put_be16(msg, ATTR_UUID_E);
 	wpabuf_put_be16(msg, WPS_UUID_LEN);
@@ -183,6 +185,8 @@ int wps_build_version(struct wpabuf *msg)
 	 * backwards compatibility reasons. The real version negotiation is
 	 * done with Version2.
 	 */
+	if (wpabuf_tailroom(msg) < 5)
+		return -1;
 	wpa_printf(MSG_DEBUG, "WPS:  * Version (hardcoded 0x10)");
 	wpabuf_put_be16(msg, ATTR_VERSION);
 	wpabuf_put_be16(msg, 1);
@@ -197,6 +201,10 @@ int wps_build_wfa_ext(struct wpabuf *msg, int req_to_enroll,
 #ifdef CONFIG_WPS2
 	u8 *len;
 
+	if (wpabuf_tailroom(msg) <
+	    7 + 3 + (req_to_enroll ? 3 : 0) +
+	    (auth_macs ? 2 + auth_macs_count * ETH_ALEN : 0))
+		return -1;
 	wpabuf_put_be16(msg, ATTR_VENDOR_EXT);
 	len = wpabuf_put(msg, 2); /* to be filled */
 	wpabuf_put_be24(msg, WPS_VENDOR_ID_WFA);
@@ -230,6 +238,8 @@ int wps_build_wfa_ext(struct wpabuf *msg, int req_to_enroll,
 
 #ifdef CONFIG_WPS_TESTING
 	if (WPS_VERSION > 0x20) {
+		if (wpabuf_tailroom(msg) < 5)
+			return -1;
 		wpa_printf(MSG_DEBUG, "WPS:  * Extensibility Testing - extra "
 			   "attribute");
 		wpabuf_put_be16(msg, ATTR_EXTENSIBILITY_TEST);