[PATCH] HID: wacom: fix OOB read in wacom_wac_pen_serial_enforce()

Wei Jie Law <[email protected]>
Newsgroups org.kernel.vger.linux-input,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
wacom_wac_pen_serial_enforce() iterates over a field's usages (up to
field->maxusage) but indexes the report bits by j * report_size:

	for (i = 0; i < report->maxfield; i++) {
		for (j = 0; j < report->field[i]->maxusage; j++) {
			...
			value = hid_field_extract(hdev, raw_data + 1,
						  offset + j * size, size);

In hid_add_field() the usage array is sized max(usage_index,
report_count), so field->maxusage can be far larger than report_count
when a descriptor lists more usages than its Report Count.  A field
declaring Usage Minimum 0 / Usage Maximum 0x2ffe with Report Count 1
gives maxusage == 12288 while the field's bit region is only 8 bits
wide.  The extract at j == 12287 then reads bit offset 12287 * 8, i.e.
byte 12287 of raw_data + 1, roughly 12 KB past a 2-byte received
report.  __extract() performs no bounds check.

The value read that way is stored into wacom_wac->serial[0] and can be
emitted to userspace as an MSC_SERIAL event by wacom_wac_pen_report(),
so this is an information disclosure and not just an out-of-bounds
read.  A malicious device only has to claim vendor id 0x056a for
hid_scan_report() to place it in HID_GROUP_WACOM and have this driver
bound to it, and a single crafted input report is enough to trigger the
read.

Clamp the inner loop to the field's actual report region with
min(maxusage, report_count), so usages that have no report data behind
them are no longer extracted.

Verified on v6.12.104, whose wacom_sys.c is identical to mainline here,
with a UHID reproducer and with an emulated USB device (raw-gadget): a
hardware watchpoint on wacom_wac->serial[0] fires with an out-of-bounds
heap byte while a 2-byte report is being processed, and no longer fires
once the loop is clamped.

Fixes: 83417206427b ("HID: wacom: Queue events with missing type/serial data for later processing")
Cc: [email protected]
Signed-off-by: Wei Jie Law <[email protected]>
---
The reproducer is available on request.

Compile-tested on bd5f485f3f02, x86_64 defconfig + CONFIG_HID_WACOM=y.

 drivers/hid/wacom_sys.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 0eafa483b7f7..1ea8763b68a8 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -113,8 +113,10 @@ static int wacom_wac_pen_serial_enforce(struct hid_device *hdev,
 
 	/* Queue events which have invalid tool type or serial number */
 	for (i = 0; i < report->maxfield; i++) {
-		for (j = 0; j < report->field[i]->maxusage; j++) {
-			struct hid_field *field = report->field[i];
+		struct hid_field *field = report->field[i];
+		unsigned int count = min(field->maxusage, field->report_count);
+
+		for (j = 0; j < count; j++) {
 			struct hid_usage *usage = &field->usage[j];
 			unsigned int equivalent_usage = wacom_equivalent_usage(usage->hid);
 			unsigned int offset;

base-commit: bd5f485f3f026225b86573e559af0b7254ef4184
-- 
2.43.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.