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

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

[Wei Jie Law]

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: stable@vger.kernel.org
Signed-off-by: Wei Jie Law <98lawweijie@gmail.com>
---
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

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] A 1-byte out-of-bounds heap read occurs in `wacom_wac_pen_serial_enforce` when processing an unnumbered HID report.
--

--- Patch [1]: [PATCH] HID: wacom: fix OOB read in wacom_wac_pen_serial_enforce() ---
commit 7eb0a702b27b2b8dba9547945aa50ecb4a4d7cc0
Author: Wei Jie Law <98lawweijie@gmail.com>

HID: wacom: fix OOB read in wacom_wac_pen_serial_enforce()

This commit fixes an out-of-bounds read in wacom_wac_pen_serial_enforce()
by clamping the usage iteration count to the field's actual report count.
This prevents extracting data beyond the received report bounds when a
descriptor lists more usages than its Report Count.

> diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
> index 0eafa483b7f75..1ea8763b68a82 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;

[Severity: High]
This isn't a bug introduced by this patch, but does the call to
hid_field_extract() later in this loop cause an out-of-bounds read if the
report is unnumbered?

Inside the loop, the code calls:

	value = hid_field_extract(hdev, raw_data+1, offset + j * size, size);

This blindly adds 1 to raw_data, assuming raw_data[0] is a report ID. If an
attacker crafts a USB HID device (or uses UHID) with the Wacom vendor ID
(0x056a) but provides a descriptor with no Report ID, the transport layer
allocates a buffer exactly matching the payload size.

For the last field in such an unnumbered report, this reads 1 byte past the
end of the heap allocation. Since the extracted value can be stored in
wacom_wac->serial[0] and emitted to userspace via MSC_SERIAL events, could
this lead to a 1-byte out-of-bounds heap read and information disclosure?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260822120926.153849-1-98lawweijie@gmail.com?part=1

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

[Wei Jie LAW]

From: Wei Jie Law <98lawweijie@gmail.com>

Superseded by v2:
https://lore.kernel.org/linux-input/20260825103104.12090-1-98lawweijie@gmail.com/

Please use that one instead.  The only difference is the Assisted-by
tags that Documentation/process/coding-assistants.rst asks for -- AI
tools were used in finding and fixing this bug and I should have
acknowledged that in the original posting.  The diff is unchanged.

Please mark this one superseded.

Thanks, and sorry for the noise,
Wei Jie