Re: [PATCH v2] HID: wacom: validate report length in wacom_intuos_pro2_bt_irq

On Fri, Aug 14, 2026 at 5:24 AM Jiri Kosina <[email protected]> wrote:
>
> Ping, Jason, could you please Ack this one? Looks good to me, but I always
> prefer to have an Ack from one of you on the Wacom HID driver.
>

LGTM. I suspect there are plenty more similar cases that need
cleaning, but this is a straightforward improvement.

Acked-by: Jason Gerecke <[email protected]>

Jason (she/they)
---
Now instead of four in the eights place /
you’ve got three, ‘Cause you added one  /
(That is to say, eight) to the two,     /
But you can’t take seven from three,    /
So you look at the sixty-fours....

> Thanks,
>
> --
> Jiri Kosina
> SUSE Labs
>
> On Mon, 13 Jul 2026, Ibrahim Hashimov wrote:
>
> > 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: [email protected]
> > Signed-off-by: Ibrahim Hashimov <[email protected]>
> > 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)
> >
>