Hi Pierre, thanks for your input. Your findings are correct. We shouldn't trust the values provided by a black-box USB device.
On Fri, 2025-05-23 at 18:49 +0200, Pierre Pronchery wrote: > Dear OpenBSD team, > > I would like to bring your attention to the following bug report from > FreeBSD, where I have ported and imported the umb(4) driver recently: > https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=284904 > > The bug report mentions: > > > in_len2mask(mask, len) will write as many as len/8 bytes: > > > > for (i = 0; i < len / 8; i++) > > p[i] = 0xff; > > > > len comes from a ipv4elem.prefixlen in a MBIM_CID_IP_CONFIGURATION > > message from the USB device, and can be any uint32_t value. So a broken > > or malicious USB device can cause a buffer overflow. > > I think that in reality, len comes from the network, which would make the > issue marginally worse. > > Can you confirm the bug on your side, and would you have any suggestion > as to how to fix it properly? > > I would suggest to make len an unsigned value in umb_decode_cid() and > subsequent calls (infolen is unsigned in the first place there) but more > importantly, to verify that prefixlen is at most 32. > > Does that make sense? > > HTH, > -- > khorben I would propose to limit 'prefixlen' to 32. Gerhard Index: sys/dev/usb/if_umb.c =================================================================== RCS file: /cvs/src/sys/dev/usb/if_umb.c,v diff -u -p -u -p -r1.59 if_umb.c --- sys/dev/usb/if_umb.c 8 Aug 2024 05:10:00 -0000 1.59 +++ sys/dev/usb/if_umb.c 26 May 2025 11:37:15 -0000 @@ -1838,7 +1838,8 @@ umb_add_inet_config(struct umb_softc *sc sin = &ifra.ifra_mask; sin->sin_family = AF_INET; sin->sin_len = sizeof (*sin); - in_len2mask(&sin->sin_addr, prefixlen); + in_len2mask(&sin->sin_addr, + MIN(prefixlen, sizeof (struct in_addr) * NBBY)); rv = in_ioctl(SIOCAIFADDR, (caddr_t)&ifra, ifp, 1); if (rv != 0) {
smime.p7s
Description: S/MIME cryptographic signature
