All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] HID: wacom: validate report length before Intuos Pro2 BT pen parse
@ 2026-07-13  7:51 Ibrahim Hashimov
  2026-07-13  8:01 ` sashiko-bot
  2026-07-13  9:34 ` [PATCH v2] HID: wacom: validate report length in wacom_intuos_pro2_bt_irq Ibrahim Hashimov
  0 siblings, 2 replies; 4+ messages in thread
From: Ibrahim Hashimov @ 2026-07-13  7:51 UTC (permalink / raw)
  To: Ping Cheng, Jason Gerecke, Jiri Kosina, Benjamin Tissoires
  Cc: linux-input, linux-kernel, stable

wacom_intuos_pro2_bt_irq() receives the wire report length in `len`
but never consults it before calling wacom_intuos_pro2_bt_pen(). The
only admission check on this path is the report-id byte:

	if (data[0] != 0x80 && data[0] != 0x81) {
		...
		return 0;
	}
	wacom_intuos_pro2_bt_pen(wacom);

wacom_intuos_pro2_bt_pen() itself does not even receive `len` (it
takes only `struct wacom_wac *wacom`), so it cannot bound its own
reads. For INTUOSP2_BT / INTUOSP2S_BT devices it unconditionally does:

	wacom->serial[0] = get_unaligned_le64(&data[99]);
	wacom->id[0]     = get_unaligned_le16(&data[107]);

i.e. it reads up to offset 108 regardless of how many bytes the
peripheral actually sent. features.type is selected from the VID/PID
id_table entry (BT_DEVICE_WACOM(0x361) -> INTUOSP2_BT) and
wacom_setup_device_quirks() force-registers the pen/pad/touch inputs
for that type independent of the report descriptor, so a malicious or
malfunctioning paired/spoofed Bluetooth peripheral can advertise that
VID/PID and send an undersized report (e.g. 10 bytes) that still
satisfies the data[0] == 0x80/0x81 gate. The driver then reads past
the received report and forwards the bytes to userspace via evdev
(MSC_SERIAL / ABS_MISC on the pen input node), an out-of-bounds read
with a concrete userspace read-back channel, and a true
out-of-bounds read on transports where the backing buffer is sized to
the (small) report descriptor rather than a fixed-size staging
buffer.

The non-Pro2 branch of the same function (INTUOSHT3_BT, reading
&data[33]/&data[41]) has the identical defect at a smaller offset.

This is the same class of bug commit 2f1763f62909 ("HID: wacom: fix
out-of-bounds read in wacom_intuos_bt_irq") already hardened in the
sibling function wacom_intuos_bt_irq(), which added explicit
short-report guards before parsing:

	case 0x04:
		if (len < 32) {
			dev_warn(..., "Report 0x04 too short: %zu bytes\n", len);
			break;
		}
		wacom_intuos_bt_process_data(wacom, data + i);
		...
	case 0x03:
		if (i == 1 && len < 22) {
			dev_warn(..., "Report 0x03 too short: %zu bytes\n", len);
			break;
		}

wacom_intuos_pro2_bt_irq() never received the analogous guard.

Fix it the same way: before calling wacom_intuos_pro2_bt_pen(), check
`len` against the minimum size each branch of that function actually
dereferences (109 bytes for INTUOSP2_BT/INTUOSP2S_BT, whose furthest
read is &data[107] as a le16 -> byte index 108; 43 bytes for the
INTUOSHT3_BT ("gen3") branch, whose furthest read is &data[41] as a
le16 -> byte index 42). On a short report, warn and bail out before
touching wacom->data past what was actually received, exactly as
wacom_intuos_bt_irq() now does for its own report ids. Returning 0
here also skips the subsequent pro2 touch/pad/battery calls for the
same malformed report, which is the safe, conservative behavior.

Runtime-verified on a v6.19 stand: injecting the truncated report
via a spoofed 056a:0361 BT peripheral leaks bytes past the declared
report length to the pen evdev node (MSC_SERIAL / ABS_MISC) before
this fix, and the same input is rejected by the new length check
with no leaked event afterward. KASAN itself stays quiet on the
fixed-size uhid staging buffer used to inject the report (the
over-read there lands in-object); on a descriptor-sized USB/BT
buffer the identical read crosses the slab boundary and is a
genuine KASAN-visible out-of-bounds read.

Fixes: 4922cd26f03c ("HID: wacom: Support 2nd-gen Intuos Pro's Bluetooth classic interface")
Cc: stable@vger.kernel.org
Signed-off-by: Ibrahim Hashimov <security@auditcode.ai>
Assisted-by: AuditCode-AI:2026.07
---
 drivers/hid/wacom_wac.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
index da1f0ea85625..89a191cf1924 100644
--- a/drivers/hid/wacom_wac.c
+++ b/drivers/hid/wacom_wac.c
@@ -1548,6 +1548,19 @@ static int wacom_intuos_pro2_bt_irq(struct wacom_wac *wacom, size_t len)
 		return 0;
 	}
 
+	if (wacom->features.type == INTUOSP2_BT ||
+	    wacom->features.type == INTUOSP2S_BT) {
+		if (len < 109) {
+			dev_warn(wacom->pen_input->dev.parent,
+				 "Pro2 BT report too short: %zu bytes\n", len);
+			return 0;
+		}
+	} else if (len < 43) {
+		dev_warn(wacom->pen_input->dev.parent,
+			 "Pro2 BT report too short: %zu bytes\n", len);
+		return 0;
+	}
+
 	wacom_intuos_pro2_bt_pen(wacom);
 	if (wacom->features.type == INTUOSP2_BT ||
 	    wacom->features.type == INTUOSP2S_BT) {
-- 
2.50.1 (Apple Git-155)


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] HID: wacom: validate report length before Intuos Pro2 BT pen parse
  2026-07-13  7:51 [PATCH] HID: wacom: validate report length before Intuos Pro2 BT pen parse Ibrahim Hashimov
@ 2026-07-13  8:01 ` sashiko-bot
  2026-07-13  9:34 ` [PATCH v2] HID: wacom: validate report length in wacom_intuos_pro2_bt_irq Ibrahim Hashimov
  1 sibling, 0 replies; 4+ messages in thread
From: sashiko-bot @ 2026-07-13  8:01 UTC (permalink / raw)
  To: Ibrahim Hashimov; +Cc: linux-input, dmitry.torokhov

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

Pre-existing issues:
- [High] The length checks introduced by the patch are insufficient. While they correctly bound the reads in `wacom_intuos_pro2_bt_pen()`, they fail to prevent out-of-bounds reads in the subsequent unconditional calls to the touch, pad, and battery parsing functions if the report length falls between the checked bounds and the maximum read offset.
--

--- Patch [1]: [PATCH] HID: wacom: validate report length before Intuos Pro2 BT pen parse ---
commit 1ba7672ecbf2a3b1f2ac28c705ac40735f4dcbcf
Author: Ibrahim Hashimov <security@auditcode.ai>

HID: wacom: validate report length before Intuos Pro2 BT pen parse

This commit adds length validation in wacom_intuos_pro2_bt_irq() to
prevent out-of-bounds reads before parsing pen reports for Intuos Pro2
and Gen3 Bluetooth devices.

> diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
> index da1f0ea85625d..89a191cf1924c 100644
> --- a/drivers/hid/wacom_wac.c
> +++ b/drivers/hid/wacom_wac.c
> @@ -1548,6 +1548,19 @@ static int wacom_intuos_pro2_bt_irq(struct wacom_wac *wacom, size_t len)
>  		return 0;
>  	}
>  
> +	if (wacom->features.type == INTUOSP2_BT ||
> +	    wacom->features.type == INTUOSP2S_BT) {
> +		if (len < 109) {
> +			dev_warn(wacom->pen_input->dev.parent,
> +				 "Pro2 BT report too short: %zu bytes\n", len);
> +			return 0;
> +		}
> +	} else if (len < 43) {
> +		dev_warn(wacom->pen_input->dev.parent,
> +			 "Pro2 BT report too short: %zu bytes\n", len);
> +		return 0;
> +	}
> +
>  	wacom_intuos_pro2_bt_pen(wacom);
>  	if (wacom->features.type == INTUOSP2_BT ||
>  	    wacom->features.type == INTUOSP2S_BT) {

[Severity: High]
This is a pre-existing issue, but are these length checks sufficient to prevent
out-of-bounds reads in the subsequent parsing functions?

While this successfully protects wacom_intuos_pro2_bt_pen(), the code goes on
to unconditionally call other parsing functions:

drivers/hid/wacom_wac.c:wacom_intuos_pro2_bt_irq() {
    ...
	wacom_intuos_pro2_bt_pen(wacom);
	if (wacom->features.type == INTUOSP2_BT ||
	    wacom->features.type == INTUOSP2S_BT) {
		wacom_intuos_pro2_bt_touch(wacom);
		wacom_intuos_pro2_bt_pad(wacom);
    ...
}

In wacom_intuos_pro2_bt_pad(), it reads up to offset 285:

drivers/hid/wacom_wac.c:wacom_intuos_pro2_bt_pad() {
    ...
	int ring = data[285] & 0x7F;
	bool ringstatus = data[285] & 0x80;
    ...
}

And for Gen3 devices, after the len < 43 check, it calls
wacom_intuos_gen3_bt_battery() which reads up to offset 45:

drivers/hid/wacom_wac.c:wacom_intuos_gen3_bt_battery() {
    ...
	bool chg = data[45] & 0x80;
	int battery_status = data[45] & 0x7F;
    ...
}

If a spoofed device sends a report that passes the initial minimum bounds
check (e.g., exactly 109 bytes for Pro2 or 43 bytes for Gen3), could this
still result in out-of-bounds reads when parsing the pad, touch, or battery
data?

>  		wacom_intuos_pro2_bt_touch(wacom);
>  		wacom_intuos_pro2_bt_pad(wacom);
>  		wacom_intuos_pro2_bt_battery(wacom);
>  	} else {
>  		wacom_intuos_gen3_bt_pad(wacom);
>  		wacom_intuos_gen3_bt_battery(wacom);
>  	}
>  	return 0;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260713075110.92469-1-security@auditcode.ai?part=1

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH v2] HID: wacom: validate report length in wacom_intuos_pro2_bt_irq
  2026-07-13  7:51 [PATCH] HID: wacom: validate report length before Intuos Pro2 BT pen parse Ibrahim Hashimov
  2026-07-13  8:01 ` sashiko-bot
@ 2026-07-13  9:34 ` Ibrahim Hashimov
  2026-07-13 10:11   ` sashiko-bot
  1 sibling, 1 reply; 4+ messages in thread
From: Ibrahim Hashimov @ 2026-07-13  9:34 UTC (permalink / raw)
  To: ping.cheng, jason.gerecke, jikos, bentiss
  Cc: dmitry.torokhov, linux-input, linux-kernel, stable

wacom_intuos_pro2_bt_irq() receives the wire report length in `len`
but never consults it before parsing. After the report-id gate it
unconditionally calls wacom_intuos_pro2_bt_pen() and then, selected by
features.type, a fixed chain of sub-parsers, none of which receive
`len`:

	wacom_intuos_pro2_bt_pen(wacom);
	if (type == INTUOSP2_BT || type == INTUOSP2S_BT) {
		wacom_intuos_pro2_bt_touch(wacom);
		wacom_intuos_pro2_bt_pad(wacom);
		wacom_intuos_pro2_bt_battery(wacom);
	} else {
		wacom_intuos_gen3_bt_pad(wacom);
		wacom_intuos_gen3_bt_battery(wacom);
	}

Each sub-parser dereferences wacom->data at fixed offsets. The furthest
byte touched on each branch is:

  INTUOSP2_BT / INTUOSP2S_BT: wacom_intuos_pro2_bt_pad() reads data[285]
	(the touchring byte), so the report must be at least 286 bytes;
  INTUOSHT3_BT ("gen3"): wacom_intuos_gen3_bt_battery() reads data[45],
	so the report must be at least 46 bytes.

features.type is selected from the VID/PID id_table entry and
wacom_setup_device_quirks() force-registers the pen/pad/touch inputs
for that type independent of the report descriptor, so a malicious or
malfunctioning paired/spoofed Bluetooth peripheral can advertise that
VID/PID and send an undersized report that still satisfies the
data[0] == 0x80/0x81 gate. The driver then reads past the received
report and forwards the bytes to userspace via evdev (MSC_SERIAL /
ABS_MISC / ABS_WHEEL on the pen and pad input nodes), an out-of-bounds
read with a concrete userspace read-back channel, and a true
out-of-bounds read on transports whose backing buffer is sized to the
(small) report descriptor rather than a fixed-size staging buffer.

This is the same class of bug commit 2f1763f62909 ("HID: wacom: fix
out-of-bounds read in wacom_intuos_bt_irq") already hardened in the
sibling wacom_intuos_bt_irq(), which guards each report id against its
minimum length before parsing.

Guard wacom_intuos_pro2_bt_irq() the same way: before parsing, reject
reports shorter than the furthest offset the selected branch actually
dereferences, warn, and bail out. Because the whole pen/touch/pad/
battery chain runs unconditionally per branch, a single up-front check
against the maximum offset (286 bytes for INTUOSP2_BT/INTUOSP2S_BT,
46 bytes for the gen3 branch) bounds every sub-parser. Returning 0 on
a short report also skips those calls for the same malformed report,
which is the safe, conservative behavior.

Fixes: 4922cd26f03c ("HID: wacom: Support 2nd-gen Intuos Pro's Bluetooth classic interface")
Cc: stable@vger.kernel.org
Signed-off-by: Ibrahim Hashimov <security@auditcode.ai>
Assisted-by: AuditCode-AI:2026.07
---
v2: the v1 check (len < 109 / len < 43) only bounded
    wacom_intuos_pro2_bt_pen(). As sashiko-bot pointed out, the
    unconditional touch/pad/battery calls read much further --
    wacom_intuos_pro2_bt_pad() up to data[285] and
    wacom_intuos_gen3_bt_battery() up to data[45] -- so a report in the
    gap (109..285 / 43..45) passed the guard yet still over-read. Raise
    the minimums to the furthest offset each branch dereferences
    (286 / 46) so the guard actually bounds the whole call chain.

 drivers/hid/wacom_wac.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
index da1f0ea85625..89a191cf1924 100644
--- a/drivers/hid/wacom_wac.c
+++ b/drivers/hid/wacom_wac.c
@@ -1548,6 +1548,19 @@ static int wacom_intuos_pro2_bt_irq(struct wacom_wac *wacom, size_t len)
 		return 0;
 	}
 
+	if (wacom->features.type == INTUOSP2_BT ||
+	    wacom->features.type == INTUOSP2S_BT) {
+		if (len < 286) {
+			dev_warn(wacom->pen_input->dev.parent,
+				 "Pro2 BT report too short: %zu bytes\n", len);
+			return 0;
+		}
+	} else if (len < 46) {
+		dev_warn(wacom->pen_input->dev.parent,
+			 "Pro2 BT report too short: %zu bytes\n", len);
+		return 0;
+	}
+
 	wacom_intuos_pro2_bt_pen(wacom);
 	if (wacom->features.type == INTUOSP2_BT ||
 	    wacom->features.type == INTUOSP2S_BT) {
-- 
2.50.1 (Apple Git-155)


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] HID: wacom: validate report length in wacom_intuos_pro2_bt_irq
  2026-07-13  9:34 ` [PATCH v2] HID: wacom: validate report length in wacom_intuos_pro2_bt_irq Ibrahim Hashimov
@ 2026-07-13 10:11   ` sashiko-bot
  0 siblings, 0 replies; 4+ messages in thread
From: sashiko-bot @ 2026-07-13 10:11 UTC (permalink / raw)
  To: Ibrahim Hashimov; +Cc: dmitry.torokhov, linux-input

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The newly added length validation uses `dev_warn()` without ratelimiting, allowing a malicious device to spam the kernel log.
--

--- Patch [1]: [PATCH v2] HID: wacom: validate report length in wacom_intuos_pro2_bt_irq ---
commit d2626ddb2110b90160eb60887108918a58cb9412
Author: Ibrahim Hashimov <security@auditcode.ai>

HID: wacom: validate report length in wacom_intuos_pro2_bt_irq

This commit adds length validation to wacom_intuos_pro2_bt_irq to prevent
out-of-bounds reads when parsing Bluetooth reports. It drops undersized
reports and logs a warning before returning.

> diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
> index da1f0ea85625d..af76e49fd0f25 100644
> --- a/drivers/hid/wacom_wac.c
> +++ b/drivers/hid/wacom_wac.c
> @@ -1548,6 +1548,19 @@ static int wacom_intuos_pro2_bt_irq(struct wacom_wac *wacom, size_t len)
>  		return 0;
>  	}
>  
> +	if (wacom->features.type == INTUOSP2_BT ||
> +	    wacom->features.type == INTUOSP2S_BT) {
> +		if (len < 286) {
> +			dev_warn(wacom->pen_input->dev.parent,
> +				 "Pro2 BT report too short: %zu bytes\n", len);

[Severity: Medium]
Since this error path is triggered by external Bluetooth input, can a
malicious or malfunctioning device flood the kernel logs by sending many
undersized reports in rapid succession?

Would it be better to use dev_warn_ratelimited() here to prevent potential
log spam and CPU/IO overhead?

> +			return 0;
> +		}
> +	} else if (len < 46) {
> +		dev_warn(wacom->pen_input->dev.parent,
> +			 "Pro2 BT report too short: %zu bytes\n", len);

[Severity: Medium]
Similarly, could this branch also benefit from dev_warn_ratelimited() to
protect against log flooding from undersized reports?

> +		return 0;
> +	}
> +
>  	wacom_intuos_pro2_bt_pen(wacom);
>  	if (wacom->features.type == INTUOSP2_BT ||
>  	    wacom->features.type == INTUOSP2S_BT) {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260713093414.94337-1-security@auditcode.ai?part=1

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-07-13 10:11 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-13  7:51 [PATCH] HID: wacom: validate report length before Intuos Pro2 BT pen parse Ibrahim Hashimov
2026-07-13  8:01 ` sashiko-bot
2026-07-13  9:34 ` [PATCH v2] HID: wacom: validate report length in wacom_intuos_pro2_bt_irq Ibrahim Hashimov
2026-07-13 10:11   ` sashiko-bot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.