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>

diff --git a/src/p2p/p2p.c b/src/p2p/p2p.c
index 7acbc7bd7e4f4b8506fe36617a7337f18fe68b23..aa1f29187bec4eb204b6dd29f0e9c89709200008 100644 (file)
--- a/src/p2p/p2p.c
+++ b/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)

diff --git a/src/p2p/p2p_build.c b/src/p2p/p2p_build.c
index 5838d35e977ee72f5d3aaba0ee9d44080d2158a5..3c819ff4d5f67925edf63dc7ef7c7e99425534c5 100644 (file)
--- a/src/p2p/p2p_build.c
+++ b/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;
 }

diff --git a/src/p2p/p2p_go_neg.c b/src/p2p/p2p_go_neg.c
index 76e7cf592320e955a09a43ebabde6d636d9a49fc..9e9d2eea4709fc5006026ee82b141733d4b161f9 100644 (file)
--- a/src/p2p/p2p_go_neg.c
+++ b/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)

diff --git a/src/p2p/p2p_i.h b/src/p2p/p2p_i.h
index 56056fa7b27b1c6798df921716ecdef241a64861..3631abd1e14938f43aea94d0209654e44b5e92c2 100644 (file)
--- a/src/p2p/p2p_i.h
+++ b/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,

diff --git a/src/wps/wps_attr_build.c b/src/wps/wps_attr_build.c
index ac9bb1e23da6f0cd61bd578e5c71c0099a5e1102..3be89453169ab4157a7f63d75511eb367232b60e 100644 (file)
--- a/src/wps/wps_attr_build.c
+++ b/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);