staging: usbip: prevent possible buffer overflow reading port records
authorMark Asselstine <asselsm@gmail.com>
Thu, 13 Feb 2014 02:47:56 +0000 (21:47 -0500)
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Fri, 14 Feb 2014 17:32:40 +0000 (09:32 -0800)
To avoid buffer overflow while reading a corrupted or possibly
modified port file we validate the length of each part of the port
record to ensure it doesn't exceed the length of the destination
buffer.

https://bugzilla.kernel.org/show_bug.cgi?id=69931

Signed-off-by: Mark Asselstine <asselsm@gmail.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
drivers/staging/usbip/userspace/libsrc/vhci_driver.c

index 209df9b37cb43f78094af0354fa3e047d0e601c3..f4bfefec5fcc729f1c5af487e03bad4d6c321bce 100644 (file)
@@ -339,27 +339,67 @@ err:
        return -1;
 }
 
-static int read_record(int rhport, char *host, char *port, char *busid)
+/*
+ * Read the given port's record.
+ *
+ * To avoid buffer overflow we will read the entire line and
+ * validate each part's size. The initial buffer is padded by 4 to
+ * accommodate the 2 spaces, 1 newline and an additional character
+ * which is needed to properly validate the 3rd part without it being
+ * truncated to an acceptable length.
+ */
+static int read_record(int rhport, char *host, unsigned long host_len,
+               char *port, unsigned long port_len, char *busid)
+
 {
+       int part;
        FILE *file;
        char path[PATH_MAX+1];
+       char *buffer, *start, *end;
+       char delim[] = {' ', ' ', '\n'};
+       int max_len[] = {(int)host_len, (int)port_len, SYSFS_BUS_ID_SIZE};
+       size_t buffer_len = host_len + port_len + SYSFS_BUS_ID_SIZE + 4;
+
+       buffer = malloc(buffer_len);
+       if (!buffer)
+               return -1;
 
        snprintf(path, PATH_MAX, VHCI_STATE_PATH"/port%d", rhport);
 
        file = fopen(path, "r");
        if (!file) {
                err("fopen");
+               free(buffer);
                return -1;
        }
 
-       if (fscanf(file, "%s %s %s\n", host, port, busid) != 3) {
-               err("fscanf");
+       if (fgets(buffer, buffer_len, file) == NULL) {
+               err("fgets");
+               free(buffer);
                fclose(file);
                return -1;
        }
-
        fclose(file);
 
+       /* validate the length of each of the 3 parts */
+       start = buffer;
+       for (part = 0; part < 3; part++) {
+               end = strchr(start, delim[part]);
+               if (end == NULL || (end - start) > max_len[part]) {
+                       free(buffer);
+                       return -1;
+               }
+               start = end + 1;
+       }
+
+       if (sscanf(buffer, "%s %s %s\n", host, port, busid) != 3) {
+               err("sscanf");
+               free(buffer);
+               return -1;
+       }
+
+       free(buffer);
+
        return 0;
 }
 
@@ -573,7 +613,8 @@ int usbip_vhci_imported_device_dump(struct usbip_imported_device *idev)
        if (idev->status == VDEV_ST_NULL || idev->status == VDEV_ST_NOTASSIGNED)
                return 0;
 
-       ret = read_record(idev->port, host, serv, remote_busid);
+       ret = read_record(idev->port, host, sizeof(host), serv, sizeof(serv),
+                         remote_busid);
        if (ret) {
                err("read_record");
                read_record_error = 1;