Windows: Fix overflow when handling HID or composite devices

* When libusb_get_device_list() is called mutliple times, the HID device
  path was unconditionally duplicated in the list of device's interfaces.
* Because array boundaries were not checked, this caused overflow and crash.
* This patch adds an out of bound check and also ensures that duplication
  of data, for HID and composite, does not occur
* It also renames the private composite_api_flags to api_flags, as well as
  reorganizes the private attributes
* Bug report and part of the fix provided by Toby Gray

diff --git a/libusb/os/windows_usb.c b/libusb/os/windows_usb.c
index 3b90d18..8615039 100644 (file)
--- a/libusb/os/windows_usb.c
+++ b/libusb/os/windows_usb.c
@@ -1149,6 +1149,8 @@ static int set_composite_interface(struct libusb_context* ctx, struct libusb_dev
        struct windows_device_priv *priv = _device_priv(dev);
        int interface_number;
 
+       if (priv->api_flags & USB_API_SET)
+               return LIBUSB_SUCCESS;
        if (priv->apib->id != USB_API_COMPOSITE) {
                usbi_err(ctx, "program assertion failed: '%s' is not composite", device_id);
                return LIBUSB_ERROR_NO_DEVICE;
@@ -1191,7 +1193,7 @@ static int set_composite_interface(struct libusb_context* ctx, struct libusb_dev
                if (priv->hid == NULL)
                        return LIBUSB_ERROR_NO_MEM;
        }
-       priv->composite_api_flags |= 1<<api;
+       priv->api_flags |= USB_API_SET | (1<<api);
 
        return LIBUSB_SUCCESS;
 }
@@ -1201,14 +1203,21 @@ static int set_hid_interface(struct libusb_context* ctx, struct libusb_device* d
 {
        struct windows_device_priv *priv = _device_priv(dev);
 
+       if (priv->api_flags & USB_API_SET)
+               return LIBUSB_SUCCESS;
        if (priv->hid == NULL) {
                usbi_err(ctx, "program assertion failed: parent is not HID");
                return LIBUSB_ERROR_NO_DEVICE;
        }
+       if (priv->hid->nb_interfaces == USB_MAXINTERFACES) {
+               usbi_err(ctx, "program assertion failed: max USB interfaces reached for HID device");
+               return LIBUSB_ERROR_NO_DEVICE;
+       }
        priv->usb_interface[priv->hid->nb_interfaces].path = dev_interface_path;
        priv->usb_interface[priv->hid->nb_interfaces].apib = &usb_api_backend[USB_API_HID];
        usbi_dbg("interface[%d] = %s", priv->hid->nb_interfaces, dev_interface_path);
        priv->hid->nb_interfaces++;
+       priv->api_flags |= USB_API_SET;
        return LIBUSB_SUCCESS;
 }
 
@@ -3983,7 +3992,7 @@ static int composite_open(struct libusb_device_handle *dev_handle)
        uint8_t flag = 1<<USB_API_WINUSB;
 
        for (api=USB_API_WINUSB; api<USB_API_MAX; api++) {
-               if (priv->composite_api_flags & flag) {
+               if (priv->api_flags & flag) {
                        r = usb_api_backend[api].open(dev_handle);
                        if (r != LIBUSB_SUCCESS) {
                                return r;
@@ -4001,7 +4010,7 @@ static void composite_close(struct libusb_device_handle *dev_handle)
        uint8_t flag = 1<<USB_API_WINUSB;
 
        for (api=USB_API_WINUSB; api<USB_API_MAX; api++) {
-               if (priv->composite_api_flags & flag) {
+               if (priv->api_flags & flag) {
                        usb_api_backend[api].close(dev_handle);
                }
                flag <<= 1;
@@ -4126,7 +4135,7 @@ static int composite_reset_device(struct libusb_device_handle *dev_handle)
        uint8_t flag = 1<<USB_API_WINUSB;
 
        for (api=USB_API_WINUSB; api<USB_API_MAX; api++) {
-               if (priv->composite_api_flags & flag) {
+               if (priv->api_flags & flag) {
                        r = usb_api_backend[api].reset_device(dev_handle);
                        if (r != LIBUSB_SUCCESS) {
                                return r;

diff --git a/libusb/os/windows_usb.h b/libusb/os/windows_usb.h
index 331f75c..cc5c398 100644 (file)
--- a/libusb/os/windows_usb.h
+++ b/libusb/os/windows_usb.h
@@ -120,6 +120,8 @@ const GUID GUID_NULL = { 0x00000000, 0x0000, 0x0000, {0x00, 0x00, 0x00, 0x00, 0x
 #define USB_API_WINUSB      3
 #define USB_API_HID         4
 #define USB_API_MAX         5
+// The following is used to indicate if the HID or composite extra props have already been set.
+#define USB_API_SET         (1<<USB_API_MAX) 
 
 #define CLASS_GUID_UNSUPPORTED      GUID_NULL
 const GUID CLASS_GUID_HID           = { 0x745A17A0, 0x74D3, 0x11D0, {0xB6, 0xFE, 0x00, 0xA0, 0xC9, 0x0F, 0x57, 0xDA} };
@@ -229,9 +231,11 @@ typedef struct libusb_device_descriptor USB_DEVICE_DESCRIPTOR, *PUSB_DEVICE_DESC
 struct windows_device_priv {
        uint8_t depth;                                          // distance to HCD
        uint8_t port;                                           // port number on the hub
+       uint8_t active_config;
+       uint8_t api_flags;                                      // HID and composite devices require additional data
        struct libusb_device *parent_dev;       // access to parent is required for usermode ops
-       char *path;                                                     // device interface path
        struct windows_usb_api_backend const *apib;
+       char *path;                                                     // device interface path
        struct {
                char *path;                                             // each interface needs a device interface path,
                struct windows_usb_api_backend const *apib; // an API backend (multiple drivers support),
@@ -240,9 +244,7 @@ struct windows_device_priv {
                bool restricted_functionality;  // indicates if the interface functionality is restricted
                                                                                // by Windows (eg. HID keyboards or mice cannot do R/W)
        } usb_interface[USB_MAXINTERFACES];
-       uint8_t composite_api_flags;            // HID and composite devices require additional data
        struct hid_device_priv *hid;
-       uint8_t active_config;
        USB_DEVICE_DESCRIPTOR dev_descriptor;
        unsigned char **config_descriptor;      // list of pointers to the cached config descriptors
 };
@@ -259,7 +261,7 @@ static inline void windows_device_priv_init(libusb_device* dev) {
        p->parent_dev = NULL;
        p->path = NULL;
        p->apib = &usb_api_backend[USB_API_UNSUPPORTED];
-       p->composite_api_flags = 0;
+       p->api_flags = 0;
        p->hid = NULL;
        p->active_config = 0;
        p->config_descriptor = NULL;

diff --git a/libusb/version_nano.h b/libusb/version_nano.h
index 7238e56..4e1e808 100644 (file)
--- a/libusb/version_nano.h
+++ b/libusb/version_nano.h
@@ -1 +1 @@
-#define LIBUSB_NANO 10539
+#define LIBUSB_NANO 10540