Hi Pierre,
On Sun, 2025-05-25 at 12:28 +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=284906 > > The bug report mentions: > > > if_umb.c calls umb_getinfobuf() with offs and size taken from messages > > sent by the USB device. The "inlen >= offs + sz" check isn't > > sufficient due to possible integer wrap. This can allow a broken or > > malicious USB device to cause a buffer overflow. > > From my reading of the current version of the umb(4) driver in OpenBSD, > ISTM that you are vulnerable to this issue as well. > > At the very least I would suggest to keep inlen and len unsigned > throughout, and then I think there should be a check for integer overflow > in umb_getinfobuf() as suggested. > > Thoughts? > > HTH, converting the variables to 64bit integer should help to prevent any overflows. And since the function is void, we should zeroize the returned data struct at all times. 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 12:31:09 -0000 @@ -1433,10 +1433,9 @@ umb_getinfobuf(void *in, int inlen, uint { offs = letoh32(offs); sz = letoh32(sz); - if (inlen >= offs + sz) { - memset(out, 0, outlen); + memset(out, 0, outlen); + if ((uint64_t)inlen >= (uint64_t)offs + (uint64_t)sz) memcpy(out, in + offs, MIN(sz, outlen)); - } } static inline int
smime.p7s
Description: S/MIME cryptographic signature
