Module Name:    src
Committed By:   martin
Date:           Thu Jan  2 09:43:56 UTC 2020

Modified Files:
        src/sys/dev/usb [netbsd-8]: ucycom.c uhid.c uthum.c

Log Message:
Pull up following revision(s) (requested by maxv in ticket #1480):

        sys/dev/usb/uthum.c: revision 1.18
        sys/dev/usb/ucycom.c: revision 1.49
        sys/dev/usb/uhid.c: revision 1.111

Fix buffer overflows. sc_{o,f}len are controlled by the USB device. By
crafting the former the device can leak stack data. By crafting the latter
the device can overwrite the stack. The combination of the two means the
device can ROP the kernel and obtain code execution (demonstrated with an
actual exploit over vHCI).

Truncate the lengths to the size of the buffers, and also drop sc_ilen
since it is unused. Patch tested with vHCI+kASan.

 -

Fix buffer overflows. Also add missing mutex_exit.

 -

Fix buffer overflows: validate the lengths at attach time, given that they
are apparently not supposed to be variable. Drop sc_ilen since it is
unused.


To generate a diff of this commit:
cvs rdiff -u -r1.45 -r1.45.8.1 src/sys/dev/usb/ucycom.c
cvs rdiff -u -r1.99 -r1.99.6.1 src/sys/dev/usb/uhid.c
cvs rdiff -u -r1.13 -r1.13.8.1 src/sys/dev/usb/uthum.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/sys/dev/usb/ucycom.c
diff -u src/sys/dev/usb/ucycom.c:1.45 src/sys/dev/usb/ucycom.c:1.45.8.1
--- src/sys/dev/usb/ucycom.c:1.45	Fri Nov 25 12:56:29 2016
+++ src/sys/dev/usb/ucycom.c	Thu Jan  2 09:43:56 2020
@@ -1,4 +1,4 @@
-/*	$NetBSD: ucycom.c,v 1.45 2016/11/25 12:56:29 skrll Exp $	*/
+/*	$NetBSD: ucycom.c,v 1.45.8.1 2020/01/02 09:43:56 martin Exp $	*/
 
 /*
  * Copyright (c) 2005 The NetBSD Foundation, Inc.
@@ -38,7 +38,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: ucycom.c,v 1.45 2016/11/25 12:56:29 skrll Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ucycom.c,v 1.45.8.1 2020/01/02 09:43:56 martin Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_usb.h"
@@ -121,11 +121,15 @@ struct ucycom_softc {
 
 	struct tty		*sc_tty;
 
+	enum {
+		UCYCOM_INIT_NONE,
+		UCYCOM_INIT_INITED
+	} sc_init_state;
+
 	kmutex_t sc_lock;	/* protects refcnt, others */
 
 	/* uhidev parameters */
 	size_t			sc_flen; /* feature report length */
-	size_t			sc_ilen; /* input report length */
 	size_t			sc_olen; /* output report length */
 
 	uint8_t			*sc_obuf;
@@ -219,13 +223,18 @@ ucycom_attach(device_t parent, device_t 
 	sc->sc_hdev.sc_intr = ucycom_intr;
 	sc->sc_hdev.sc_parent = uha->parent;
 	sc->sc_hdev.sc_report_id = uha->reportid;
+	sc->sc_init_state = UCYCOM_INIT_NONE;
 
 	uhidev_get_report_desc(uha->parent, &desc, &size);
 	repid = uha->reportid;
-	sc->sc_ilen = hid_report_size(desc, size, hid_input, repid);
 	sc->sc_olen = hid_report_size(desc, size, hid_output, repid);
 	sc->sc_flen = hid_report_size(desc, size, hid_feature, repid);
 
+	if (sc->sc_olen != 8 && sc->sc_olen != 32)
+		return;
+	if (sc->sc_flen != 5)
+		return;
+
 	sc->sc_msr = sc->sc_mcr = 0;
 
 	/* set up tty */
@@ -238,6 +247,8 @@ ucycom_attach(device_t parent, device_t 
 
 	/* Nothing interesting to report */
 	aprint_normal("\n");
+
+	sc->sc_init_state = UCYCOM_INIT_INITED;
 }
 
 
@@ -334,10 +345,10 @@ ucycomopen(dev_t dev, int flag, int mode
 
 	if (sc == NULL)
 		return ENXIO;
-
 	if (sc->sc_dying)
 		return EIO;
-
+	if (sc->sc_init_state != UCYCOM_INIT_INITED)
+		return ENXIO;
 	if (!device_is_active(sc->sc_hdev.sc_dev))
 		return ENXIO;
 

Index: src/sys/dev/usb/uhid.c
diff -u src/sys/dev/usb/uhid.c:1.99 src/sys/dev/usb/uhid.c:1.99.6.1
--- src/sys/dev/usb/uhid.c:1.99	Sat Mar 11 12:41:14 2017
+++ src/sys/dev/usb/uhid.c	Thu Jan  2 09:43:56 2020
@@ -1,4 +1,4 @@
-/*	$NetBSD: uhid.c,v 1.99 2017/03/11 12:41:14 maya Exp $	*/
+/*	$NetBSD: uhid.c,v 1.99.6.1 2020/01/02 09:43:56 martin Exp $	*/
 
 /*
  * Copyright (c) 1998, 2004, 2008, 2012 The NetBSD Foundation, Inc.
@@ -35,7 +35,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: uhid.c,v 1.99 2017/03/11 12:41:14 maya Exp $");
+__KERNEL_RCSID(0, "$NetBSD: uhid.c,v 1.99.6.1 2020/01/02 09:43:56 martin Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_compat_netbsd.h"
@@ -393,6 +393,8 @@ uhid_do_read(struct uhid_softc *sc, stru
 	if (sc->sc_state & UHID_IMMED) {
 		DPRINTFN(1, ("uhidread immed\n"));
 		extra = sc->sc_hdev.sc_report_id != 0;
+		if (sc->sc_isize + extra > sizeof(buffer))
+			return ENOBUFS;
 		err = uhidev_get_report(&sc->sc_hdev, UHID_INPUT_REPORT,
 					buffer, sc->sc_isize + extra);
 		if (err)
@@ -536,8 +538,10 @@ uhid_do_ioctl(struct uhid_softc *sc, u_l
 	case FIOASYNC:
 		mutex_enter(proc_lock);
 		if (*(int *)addr) {
-			if (sc->sc_async != NULL)
+			if (sc->sc_async != NULL) {
+				mutex_exit(proc_lock);
 				return EBUSY;
+			}
 			sc->sc_async = l->l_proc;
 			DPRINTF(("uhid_do_ioctl: FIOASYNC %p\n", l->l_proc));
 		} else
@@ -584,6 +588,8 @@ uhid_do_ioctl(struct uhid_softc *sc, u_l
 	case USB_SET_IMMED:
 		if (*(int *)addr) {
 			extra = sc->sc_hdev.sc_report_id != 0;
+			if (sc->sc_isize + extra > sizeof(buffer))
+				return ENOBUFS;
 			err = uhidev_get_report(&sc->sc_hdev, UHID_INPUT_REPORT,
 						buffer, sc->sc_isize + extra);
 			if (err)
@@ -610,6 +616,8 @@ uhid_do_ioctl(struct uhid_softc *sc, u_l
 			return EINVAL;
 		}
 		extra = sc->sc_hdev.sc_report_id != 0;
+		if (size + extra > sizeof(re->ucr_data))
+			return ENOBUFS;
 		err = uhidev_get_report(&sc->sc_hdev, re->ucr_report,
 		    re->ucr_data, size + extra);
 		if (extra)
@@ -633,6 +641,8 @@ uhid_do_ioctl(struct uhid_softc *sc, u_l
 		default:
 			return EINVAL;
 		}
+		if (size > sizeof(re->ucr_data))
+			return ENOBUFS;
 		err = uhidev_set_report(&sc->sc_hdev, re->ucr_report,
 		    re->ucr_data, size);
 		if (err)

Index: src/sys/dev/usb/uthum.c
diff -u src/sys/dev/usb/uthum.c:1.13 src/sys/dev/usb/uthum.c:1.13.8.1
--- src/sys/dev/usb/uthum.c:1.13	Fri Nov 25 12:56:29 2016
+++ src/sys/dev/usb/uthum.c	Thu Jan  2 09:43:56 2020
@@ -1,4 +1,4 @@
-/*	$NetBSD: uthum.c,v 1.13 2016/11/25 12:56:29 skrll Exp $   */
+/*	$NetBSD: uthum.c,v 1.13.8.1 2020/01/02 09:43:56 martin Exp $   */
 /*	$OpenBSD: uthum.c,v 1.6 2010/01/03 18:43:02 deraadt Exp $   */
 
 /*
@@ -22,7 +22,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: uthum.c,v 1.13 2016/11/25 12:56:29 skrll Exp $");
+__KERNEL_RCSID(0, "$NetBSD: uthum.c,v 1.13.8.1 2020/01/02 09:43:56 martin Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_usb.h"
@@ -82,7 +82,6 @@ struct uthum_softc {
 
 	/* uhidev parameters */
 	size_t			 sc_flen;	/* feature report length */
-	size_t			 sc_ilen;	/* input report length */
 	size_t			 sc_olen;	/* output report length */
 
 	/* sensor framework */
@@ -148,7 +147,6 @@ uthum_attach(device_t parent, device_t s
 
 	uhidev_get_report_desc(uha->parent, &desc, &size);
 	repid = uha->reportid;
-	sc->sc_ilen = hid_report_size(desc, size, hid_input, repid);
 	sc->sc_olen = hid_report_size(desc, size, hid_output, repid);
 	sc->sc_flen = hid_report_size(desc, size, hid_feature, repid);
 
@@ -263,33 +261,36 @@ uthum_read_data(struct uthum_softc *sc, 
 {
 	int i;
 	uint8_t cmdbuf[32], report[256];
+	size_t olen, flen;
 
 	/* if return buffer is null, do nothing */
 	if ((buf == NULL) || len == 0)
 		return 0;
 
+	olen = uimin(sc->sc_olen, sizeof(cmdbuf));
+
 	/* issue query */
 	memset(cmdbuf, 0, sizeof(cmdbuf));
 	memcpy(cmdbuf, cmd_start, sizeof(cmd_start));
 	if (uhidev_set_report(&sc->sc_hdev, UHID_OUTPUT_REPORT,
-	    cmdbuf, sc->sc_olen))
+	    cmdbuf, olen))
 		return EIO;
 
 	memset(cmdbuf, 0, sizeof(cmdbuf));
 	cmdbuf[0] = target_cmd;
 	if (uhidev_set_report(&sc->sc_hdev, UHID_OUTPUT_REPORT,
-	    cmdbuf, sc->sc_olen))
+	    cmdbuf, olen))
 		return EIO;
 
 	memset(cmdbuf, 0, sizeof(cmdbuf));
 	for (i = 0; i < 7; i++) {
 		if (uhidev_set_report(&sc->sc_hdev, UHID_OUTPUT_REPORT,
-		    cmdbuf, sc->sc_olen))
+		    cmdbuf, olen))
 			return EIO;
 	}
 	memcpy(cmdbuf, cmd_end, sizeof(cmd_end));
 	if (uhidev_set_report(&sc->sc_hdev, UHID_OUTPUT_REPORT,
-	    cmdbuf, sc->sc_olen))
+	    cmdbuf, olen))
 		return EIO;
 
 	/* wait if required */
@@ -297,8 +298,9 @@ uthum_read_data(struct uthum_softc *sc, 
 		tsleep(&sc->sc_sme, 0, "uthum", (need_delay*hz+999)/1000 + 1);
 
 	/* get answer */
+	flen = uimin(sc->sc_flen, sizeof(report));
 	if (uhidev_get_report(&sc->sc_hdev, UHID_FEATURE_REPORT,
-	    report, sc->sc_flen))
+	    report, flen))
 		return EIO;
 	memcpy(buf, report, len);
 	return 0;

Reply via email to