Correct the length of the usb_string_descriptor_t and fix a possible

buffer overflow.

Problem found by (and original diff from) Marc Winiger <mw@msys.ch>, comment
by me on jsg's request.

ok jsg, claudio

2 files changed, 16 insertions, 7 deletions

diff --git a/sys/dev/usb/usb.h b/sys/dev/usb/usb.h
index ac1c866d202..78439b78b09 100644
--- a/sys/dev/usb/usb.h
+++ b/sys/dev/usb/usb.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: usb.h,v 1.28 2007/06/17 07:53:11 mbalmer Exp $ */
+/*	$OpenBSD: usb.h,v 1.29 2007/07/27 09:16:09 mbalmer Exp $ */
 /*	$NetBSD: usb.h,v 1.69 2002/09/22 23:20:50 augustss Exp $	*/
 /*	$FreeBSD: src/sys/dev/usb/usb.h,v 1.14 1999/11/17 22:33:46 n_hibma Exp $	*/
 
@@ -251,12 +251,21 @@ typedef struct {
 } __packed usb_endpoint_descriptor_t;
 #define USB_ENDPOINT_DESCRIPTOR_SIZE 7
 
+/*
+ * Note: The length of the USB string descriptor is stored in a one byte
+ * value and can therefore be no longer than 255 bytes.  Two bytes are
+ * used for the length itself and the descriptor type, a theoretical maximum
+ * of 253 bytes is left for the actual string data.  Since the strings are
+ * encoded as 2-byte unicode characters, only 252 bytes or 126 two-byte
+ * characters can be used.  USB_MAX_STRING_LEN is defined as 127, leaving
+ * space for the terminal '\0' character in C strings.
+ */
 typedef struct {
 	uByte		bLength;
 	uByte		bDescriptorType;
-	uWord		bString[127];
+	uWord		bString[126];
 } __packed usb_string_descriptor_t;
-#define USB_MAX_STRING_LEN 128
+#define USB_MAX_STRING_LEN 127
 #define USB_LANGUAGE_TABLE 0	/* # of the string language id table */
 
 /* Hub specific request */
diff --git a/sys/dev/usb/usbf_subr.c b/sys/dev/usb/usbf_subr.c
index 6971f1d8582..3fc93cf128f 100644
--- a/sys/dev/usb/usbf_subr.c
+++ b/sys/dev/usb/usbf_subr.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: usbf_subr.c,v 1.9 2007/06/15 11:41:48 mbalmer Exp $	*/
+/*	$OpenBSD: usbf_subr.c,v 1.10 2007/07/27 09:16:09 mbalmer Exp $	*/
 
 /*
  * Copyright (c) 2006 Uwe Stuehler <uwe@openbsd.org>
@@ -308,8 +308,8 @@ usbf_add_string(usbf_device_handle dev, const char *string)
 	    dev->string_id == USBF_STRING_ID_MAX)
 		return USBF_EMPTY_STRING_ID;
 
-	if ((len = strlen(string)) > USB_MAX_STRING_LEN)
-		len = USB_MAX_STRING_LEN;
+	if ((len = strlen(string)) >= USB_MAX_STRING_LEN)
+		len = USB_MAX_STRING_LEN - 1;
 
 	oldsize = dev->sdesc_size;
 	newsize = oldsize + 2 + 2 * len;
@@ -322,7 +322,7 @@ usbf_add_string(usbf_device_handle dev, const char *string)
 	sd = (usb_string_descriptor_t *)((char *)sd + oldsize);
 	sd->bLength = newsize - oldsize;
 	sd->bDescriptorType = UDESC_STRING;
-	for (i = 0; string[i] != '\0'; i++)
+	for (i = 0; string[i] != '\0' && i < len; i++)
 		USETW(sd->bString[i], string[i]);
 
 	id = dev->string_id++;