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

3 files changed, 20 insertions, 9 deletions

diff --git a/libusb/os/windows_usb.c b/libusb/os/windows_usb.c
index 3b90d18..8615039 100644
--- 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
--- 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
--- a/libusb/version_nano.h
+++ b/libusb/version_nano.h
@@ -1 +1 @@
-#define LIBUSB_NANO 10539
+#define LIBUSB_NANO 10540