Re: [PATCH 01/14] HID: validate HID report id size

On Thu, Aug 29, 2013 at 11:03 AM, Jiri Kosina [off-list ref] wrote:

On Wed, 28 Aug 2013, Jiri Kosina wrote:

quoted

From: Kees Cook <redacted>

The "Report ID" field of a HID report is used to build indexes of
reports. The kernel's index of these is limited to 256 entries, so any
malicious device that sets a Report ID greater than 255 will trigger
memory corruption on the host:

[ 1347.156239] BUG: unable to handle kernel paging request at ffff88094958a878
[ 1347.156261] IP: [<ffffffff813e4da0>] hid_register_report+0x2a/0x8b

CVE-2013-2888

Signed-off-by: Kees Cook <redacted>
Cc: stable@kernel.org

Applied this one to hid.git#for-3.11/CVE-2013-2888

Well, I am a little bit lost here. I know the patch will not break any
existing things, but still, I really can not understand how this can
happen:

- Report IDs are specifically specified to be 1-byte fields. That
means that the max they can have is 0xFF, which is 255 :-/
- If I look at the implementation, hid_get_report() is where we fetch
the report ID from the data. But the report ID is declared as an u8...
so with a max of 255.

So the only problem might comes when specific hid drivers call
manually hid_register_report(), and in this case, we can easily
prevent this by looking at their code. No? Or we could also
specifically change the type of the argument id in
hid_register_report() to be an u8.

Basically, I'd be eager to have proper hid report descriptors and
events that can produce them, so I can add them to my HID regressions
test suite.

Cheers,
Benjamin

Thanks,

--
Jiri Kosina
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html