linux-kernel - Re: [PATCH] HID: usbhid: fix out-of-bounds bug

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-next>] [day] [month] [year] [list]

Message-ID: <Pine.LNX.4.44L0.1709271013300.1390-100000@iolanthe.rowland.org>
Date:   Wed, 27 Sep 2017 10:29:19 -0400 (EDT)
From:   Alan Stern <stern@...land.harvard.edu>
To:     Michel Hermier <michel.hermier@...il.com>
cc:     Kostya Serebryany <kcc@...gle.com>,
        syzkaller <syzkaller@...glegroups.com>,
        Jiri Kosina <jikos@...nel.org>,
        USB list <linux-usb@...r.kernel.org>,
        Jaejoong Kim <climbbb.kim@...il.com>,
        Andrey Konovalov <andreyknvl@...gle.com>,
        Benjamin Tissoires <benjamin.tissoires@...hat.com>,
        Dmitry Vyukov <dvyukov@...gle.com>,
        <linux-input@...r.kernel.org>, LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] HID: usbhid: fix out-of-bounds bug

On Wed, 27 Sep 2017, Michel Hermier wrote:

> Le 27 sept. 2017 07:42, "Alan Stern" <stern@...land.harvard.edu> a écrit :

> > -       for (n = 0; n < hdesc->bNumDescriptors; n++)
> > +       num_descriptors = min_t(int, hdesc->bNumDescriptors,
> > +                               (hdesc->bLength - 6) / 3);
> > +       for (n = 0; n < num_descriptors; n++)
> >                 if (hdesc->desc[n].bDescriptorType == HID_DT_REPORT)
> >                         rsize = le16_to_cpu(hdesc->desc[n].
> wDescriptorLength);
> 
> Yes, this is a lot better.
> 
> 
> Is it possible to explicit the magic number 6 and 3 in the code. Currently,
> it looks like it comes from no where.

Yes, it is possible.  The 6 is equal to

	offsetof(struct hid_descriptor, desc)

and the 3 is equal to

	sizeof(struct hid_class_descriptor)

(at least, I think it is -- the structure is marked as packed so its 
size should be 3).

In this case I found the numbers to be more readable, but other people 
may have different opinions.

> I'm also wondering if this change will not affect some devices in the wild,
> by rejecting hid descriptors with num descriptors == 0 ?

It's possible, but I doubt it.  If such devices do exist, they should
never have worked in the first place.  Certainly they would generate
warnings or errors during enumeration because of their invalid
descriptors.

Alan Stern