From 27acc75eb27ce7e4a090279c98ecfa53b9a07bf8 Mon Sep 17 00:00:00 2001 From: Martin Pieuchot Date: Fri, 14 Feb 2020 14:55:31 +0000 Subject: Prevent buffer overflows by not assuming the report length, given by the hardware, is necessarily smaller than the length of the on-stack buffer. Original fix from Maxime Villard in NetBSD via deraadt@. Tested by matthieu@ --- sys/dev/usb/uthum.c | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) (limited to 'sys/dev/usb') diff --git a/sys/dev/usb/uthum.c b/sys/dev/usb/uthum.c index 4af3648e402..601abdc64ab 100644 --- a/sys/dev/usb/uthum.c +++ b/sys/dev/usb/uthum.c @@ -1,4 +1,4 @@ -/* $OpenBSD: uthum.c,v 1.33 2020/01/13 10:01:24 mpi Exp $ */ +/* $OpenBSD: uthum.c,v 1.34 2020/02/14 14:55:30 mpi Exp $ */ /* * Copyright (c) 2009, 2010 Yojiro UO @@ -284,28 +284,29 @@ int uthum_issue_cmd(struct uthum_softc *sc, uint8_t target_cmd, int delay) { uint8_t cmdbuf[32]; - int i, actlen; + int i, actlen, olen; + + olen = MIN(sc->sc_olen, sizeof(cmdbuf)); bzero(cmdbuf, sizeof(cmdbuf)); memcpy(cmdbuf, cmd_issue, sizeof(cmd_issue)); actlen = uhidev_set_report(sc->sc_hdev.sc_parent, UHID_OUTPUT_REPORT, - sc->sc_hdev.sc_report_id, cmdbuf, sc->sc_olen); - if (actlen != sc->sc_olen) + sc->sc_hdev.sc_report_id, cmdbuf, olen); + if (actlen != olen) return EIO; bzero(cmdbuf, sizeof(cmdbuf)); cmdbuf[0] = target_cmd; actlen = uhidev_set_report(sc->sc_hdev.sc_parent, UHID_OUTPUT_REPORT, - sc->sc_hdev.sc_report_id, cmdbuf, sc->sc_olen); - if (actlen != sc->sc_olen) + sc->sc_hdev.sc_report_id, cmdbuf, olen); + if (actlen != olen) return EIO; bzero(cmdbuf, sizeof(cmdbuf)); for (i = 0; i < 7; i++) { actlen = uhidev_set_report(sc->sc_hdev.sc_parent, - UHID_OUTPUT_REPORT, sc->sc_hdev.sc_report_id, cmdbuf, - sc->sc_olen); - if (actlen != sc->sc_olen) + UHID_OUTPUT_REPORT, sc->sc_hdev.sc_report_id, cmdbuf, olen); + if (actlen != olen) return EIO; } @@ -322,6 +323,7 @@ uthum_read_data(struct uthum_softc *sc, uint8_t target_cmd, uint8_t *buf, size_t len, int delay) { uint8_t cmdbuf[32], report[256]; + int olen, flen; /* if return buffer is null, do nothing */ if ((buf == NULL) || len == 0) @@ -330,10 +332,12 @@ uthum_read_data(struct uthum_softc *sc, uint8_t target_cmd, uint8_t *buf, if (uthum_issue_cmd(sc, target_cmd, 50)) return 0; + olen = MIN(sc->sc_olen, sizeof(cmdbuf)); + bzero(cmdbuf, sizeof(cmdbuf)); memcpy(cmdbuf, cmd_query, sizeof(cmd_query)); if (uhidev_set_report(sc->sc_hdev.sc_parent, UHID_OUTPUT_REPORT, - sc->sc_hdev.sc_report_id, cmdbuf, sc->sc_olen) != sc->sc_olen) + sc->sc_hdev.sc_report_id, cmdbuf, olen) != olen) return EIO; /* wait if required */ @@ -342,8 +346,9 @@ uthum_read_data(struct uthum_softc *sc, uint8_t target_cmd, uint8_t *buf, MSEC_TO_NSEC(delay)); /* get answer */ + flen = MIN(sc->sc_flen, sizeof(report)); if (uhidev_get_report(sc->sc_hdev.sc_parent, UHID_FEATURE_REPORT, - sc->sc_hdev.sc_report_id, report, sc->sc_flen) != sc->sc_flen) + sc->sc_hdev.sc_report_id, report, flen) != flen) return EIO; memcpy(buf, report, len); return 0; -- cgit v1.2.3