Index | Thread | Search

From:
"Osvaldo J. Filho" <janeri@gmail.com>
Subject:
avoid 32-bit overflow in HID report-size computation (sys/dev/hid/hid.c)
To:
tech@openbsd.org
Date:
Sun, 18 Oct 2026 20:37:49 -0300

Download raw body.

Thread
Hi,

Auditing the HID report-descriptor parser I noticed hid_report_size() and
the position accumulation in hid_get_item() compute loc.pos + loc.size *
loc.count with uint32_t operands from the report descriptor, unbounded.  A
crafted descriptor wraps the 32-bit multiply, so
hid_report_size() returns a wrong (usually too small) size.

I don't think it is exploitable: hid_get_data() clamps reads to the
caller's buffer and the drivers reject reports larger than the endpoint.
The size is still wrong, so do the math in 64-bit and saturate.

Diff is against sys/dev/hid/hid.c in -current.

ok?

--- a/sys/dev/hid/hid.c 2026-10-18 23:25:25.147558000 +0000
+++ b/sys/dev/hid/hid.c 2026-10-18 23:25:25.170985000 +0000
@@ -230,10 +230,15 @@
                 * if correct kind!
                 */
                if (s->kind == hid_all || s->kind == c->kind) {
+                       uint64_t end;
+
                        *h = *c;
                        DPRINTF("%u,%u,%u\n", h->loc.pos,
                            h->loc.size, h->loc.count);
-                       c->loc.pos += c->loc.size * c->loc.count;
+                       end = (uint64_t)c->loc.pos +
+                           (uint64_t)c->loc.size * c->loc.count;
+                       c->loc.pos = end > 0xffffffffU ? 0xffffffffU :
+                           (uint32_t)end;
                        return (1);
                }
        }
@@ -512,7 +517,7 @@
 {
        struct hid_data *d;
        struct hid_item h;
-       int lo, hi;
+       int64_t lo, hi;

        h.report_ID = 0;
        lo = hi = -1;
@@ -531,8 +536,11 @@
                                }
 #endif
                        }
-                       hi = h.loc.pos + h.loc.size * h.loc.count;
-                       DPRINTF("hid_report_size: lo=%d hi=%d\n", lo, hi);
+                       hi = (uint64_t)h.loc.pos +
+                           (uint64_t)h.loc.size * h.loc.count;
+                       if (hi > 0xffffffffU)
+                               hi = 0xffffffffU;
+                       DPRINTF("hid_report_size: lo=%d hi=%d\n", (int)lo,
(int)hi);

                }
        }


Osvaldo Janeri Filho