Index | Thread | Search

From:
"Janeri Filho, Osvaldo" <Osvaldo.JaneriFilho@amd.com>
Subject:
avoid 32-bit overflow in HID report-size computation (sys/dev/hid/hid.c)
To:
"tech@openbsd.org" <tech@openbsd.org>
Date:
Sun, 18 Oct 2026 23:33:42 +0000

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
<ojanerif@amd.com>