linux-kernel - Re: [PATCH] HID: validate report length and constants

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <b3131f6c322ac4c62c4b00142b55fde7@davidebeatrici.dev>
Date: Thu, 04 Dec 2025 20:48:50 +0100
From: Davide Beatrici <me@...idebeatrici.dev>
To: Benjamin Tissoires <bentiss@...nel.org>
Cc: Terry Junge <linuxhid@...micgizmosystems.com>,
 linux-kernel@...r.kernel.org, linux-input@...r.kernel.org, jikos@...nel.org,
 benjamin.tissoires@...hat.com
Subject: Re: [PATCH] HID: validate report length and constants

> However, the device also shows an output report of size 1, but it is 
> not
> supposed to send it as an input report. I wonder if the firmware bug is
> not that it tries to give the host the current state of its output
> report at plug (which is wrong but Windows must be papering over it).

On that note, I noticed the malformed packet is not sent upon 
reconnecting the
device if it's been plugged in for some time.
When that happens I can reproduce the issue by enabling a wireless mode 
through
the switch on the bottom and wait a bit before disabling it and 
connecting the
device again.
I suspect the battery gets slightly discharged and the device sends a
"charging complete" signal or something.

> - the URB is of size 1, so the fact that the constant field is not 0
> means that we are just reading random memory at offset 1 in the
> provided data, so you might have a chance that it eventually becomes 0

Good point, if it's effectively random memory we cannot rely on that.

> - the fix should be focusing on the length of the provided report, not
> on the content. However, in hid_report_raw_event(), just before you
> inserted your call to your hid_validate_report(), there is already a
> check on the length of the report which memsets to 0 the rest of the
> buffer. This seems a little bit optimistic if the provided buffer from
> USB is exactly the size of the provided "size" argument.
> But then, why would you get random data in the const fields if there
> is a memset if the provided length is "1"?
> 
> So, can you add a printk before your call to hid_validate_report() to
> show the provided "size" argument (csize), or just enable the hid_dbg()
> trace output which should tell us if we enter that test and do the
> memset (which I suppose we are not).

report 8 has csize=16 rsize=16
report 0 has csize=1 rsize=8
report 0 is too short, (1 < 8)

Which means we do enter the test and execute the memset()...