Vadim Zhukov wrote: > 2016-04-30 7:38 GMT+03:00 Jonathan Gray <[email protected]>: > > On Wed, Apr 27, 2016 at 07:49:50PM -0700, Geoff Hill wrote: > >> Fix possible reads past the end of the buffer. > >> > >> Found by random fuzz testing (zzuf). Without the fix the fuzzer crashes > >> in several seconds; with the patch, the fuzzer runs clean for hours. > > > > Any reason to not replace the somewhat arbitary earlier test > > for this? > > > > Index: midiplay.c > > =================================================================== > > RCS file: /cvs/src/usr.bin/midiplay/midiplay.c,v > > retrieving revision 1.17 > > diff -u -p -U9 -r1.17 midiplay.c > > --- midiplay.c 8 Feb 2015 23:40:34 -0000 1.17 > > +++ midiplay.c 30 Apr 2016 04:35:31 -0000 > > @@ -306,19 +306,19 @@ playdata(u_char *buf, u_int tot, char *n > > tracks = calloc(ntrks, sizeof(struct track)); > > if (tracks == NULL) > > err(1, "malloc() tracks failed"); > > for (t = 0; t < ntrks; ) { > > if (p >= end - MARK_LEN - SIZE_LEN) { > > warnx("Cannot find track %d", t); > > goto ret; > > } > > len = GET32(p + MARK_LEN); > > - if (len > 1000000) { /* a safe guard */ > > + if (p + MARK_LEN + SIZE_LEN + len > end) { > > It's better to avoid "+" in checks to avoid overflow, no?
Yeah, I'm pretty sure the above overflow check is undefined.
