commit ba8bd54cda59bb55e62a390b7fd0bcafcc72c034
author David Gilhooley <dgilhooley@google.com> Sat May 02 01:08:23 2020 +0000
committer CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org> Sat May 02 01:08:23 2020 +0000
tree a25266639edb34c73d25b2b081e43f15157f43ba
parent 144c9e145fa65770314dd75d79de41c62719555e

[hid] Fix GetReport buffer overrun

Fix a potential buffer overrun in GetReport where
a malicious USB device could have a report size
greater than HID_MAX_REPORT_LEN.

Bug 50622: Fixed

Change-Id: I7cf36d004dcae7bdfffa5d1a355279fae7efe500
Reviewed-on: https://fuchsia-review.googlesource.com/c/fuchsia/+/383179
Commit-Queue: David Gilhooley <dgilhooley@google.com>
Reviewed-by: Suraj Malhotra <surajmalhotra@google.com>
Reviewed-by: Andres Oportus <andresoportus@google.com>
Testability-Review: Suraj Malhotra <surajmalhotra@google.com>
Testability-Review: Andres Oportus <andresoportus@google.com>

src/ui/input/drivers/hid/hid-test.cc

diff --git a/src/ui/input/drivers/hid/hid-test.cc b/src/ui/input/drivers/hid/hid-test.cc
index 59be0e5..7da71a8 100644
--- a/src/ui/input/drivers/hid/hid-test.cc
+++ b/src/ui/input/drivers/hid/hid-test.cc
@@ -693,4 +693,37 @@
   ASSERT_BYTES_EQ(&feature_report, &received_report, actual);
 }
 
+// Tests that a device with too large reports don't cause buffer overruns.
+TEST_F(HidDeviceTest, GetReportBufferOverrun) {
+  const uint8_t desc[] = {
+      0x05, 0x01,                    // Usage Page (Generic Desktop Ctrls)
+      0x09, 0x02,                    // Usage (Mouse)
+      0xA1, 0x01,                    // Collection (Application)
+      0x05, 0x09,                    //   Usage Page (Button)
+      0x09, 0x30,                    //   Usage (0x30)
+      0x97, 0x00, 0xF0, 0x00, 0x00,  //   Report Count (65279)
+      0x75, 0x08,                    //   Report Size (8)
+      0x25, 0x01,                    //   Logical Maximum (1)
+      0x81, 0x02,  //   Input (Data,Var,Abs,No Wrap,Linear,Preferred State,No Null Position)
+      0xC0,        // End Collection
+
+      // 22 bytes
+  };
+  size_t desc_size = sizeof(desc);
+  fake_hidbus_.SetDescriptor(desc, desc_size);
+
+  hid_info_t info = {};
+  info.device_class = HID_DEVICE_CLASS_OTHER;
+  info.boot_device = false;
+  fake_hidbus_.SetHidInfo(info);
+
+  ASSERT_OK(device_->Bind(fake_hidbus_.GetProto()));
+
+  std::vector<uint8_t> report(0xFF0000);
+  size_t actual;
+  ASSERT_EQ(
+      device_->HidDeviceGetReport(HID_REPORT_TYPE_INPUT, 0, report.data(), report.size(), &actual),
+      ZX_ERR_INTERNAL);
+}
+
 }  // namespace hid_driver

src/ui/input/drivers/hid/hid.cc

diff --git a/src/ui/input/drivers/hid/hid.cc b/src/ui/input/drivers/hid/hid.cc
index 95399cb..5224e08 100644
--- a/src/ui/input/drivers/hid/hid.cc
+++ b/src/ui/input/drivers/hid/hid.cc
@@ -303,6 +303,11 @@
   if (needed > report_count) {
     return ZX_ERR_BUFFER_TOO_SMALL;
   }
+  if (needed > HID_MAX_REPORT_LEN) {
+    zxlogf(ERROR, "hid: GetReport: Report size 0x%lx larger than max size 0x%x", needed,
+           HID_MAX_REPORT_LEN);
+    return ZX_ERR_INTERNAL;
+  }
 
   uint8_t report[HID_MAX_REPORT_LEN];
   size_t actual = 0;