On Mon, Mar 23, 2020 at 05:52:01PM -0700, John Rummell wrote: > Chromium fuzzers have caught places where uninitialized data was used due > to calls to avio_read() not verifying that the number of bytes expected was > actually read. So updating the code to check the result from avio_read().
> mov.c | 26 ++++++++++++++++++-------- > oggdec.c | 3 ++- > wavdec.c | 12 ++++++++---- > 3 files changed, 28 insertions(+), 13 deletions(-) > 66938bc5adfc7d151b376f6d396c4a0dc7f97a4c > 0001-Check-return-value-from-avio_read-to-verify-data-act.patch > From 7f80d50711486a4b923bd8d1e26abc9649d570e3 Mon Sep 17 00:00:00 2001 > From: John Rummell <jrumm...@chromium.org> > Date: Mon, 23 Mar 2020 15:48:33 -0700 > Subject: [PATCH] Check return value from avio_read() to verify data actually > read > > If the buffer doesn't contain enough bytes when reading a stream, > fail rather than continuing on with unitialized data. One attempt > caught by Chromium fuzzers (crbug.com/1054229), rest done by looking > for calls to avio_read() that don't check the result in Chromium > code search. > --- > libavformat/mov.c | 26 ++++++++++++++++++-------- > libavformat/oggdec.c | 3 ++- > libavformat/wavdec.c | 12 ++++++++---- > 3 files changed, 28 insertions(+), 13 deletions(-) > > diff --git a/libavformat/mov.c b/libavformat/mov.c > index f280f360b6..a5b4d04e37 100644 > --- a/libavformat/mov.c > +++ b/libavformat/mov.c > @@ -1012,10 +1012,16 @@ static int mov_read_adrm(MOVContext *c, AVIOContext > *pb, MOVAtom atom) > } > > /* drm blob processing */ > - avio_read(pb, output, 8); // go to offset 8, absolute position 0x251 > - avio_read(pb, input, DRM_BLOB_SIZE); > - avio_read(pb, output, 4); // go to offset 4, absolute position 0x28d > - avio_read(pb, file_checksum, 20); > + /* go to offset 8, absolute position 0x251 */ > + if ((ret = avio_read(pb, output, 8)) != 8) > + goto fail; > + if ((ret = avio_read(pb, input, DRM_BLOB_SIZE)) != DRM_BLOB_SIZE) > + goto fail; > + /* go to offset 4, absolute position 0x28d */ > + if ((ret = avio_read(pb, output, 4)) != 4) > + goto fail; > + if ((ret = avio_read(pb, file_checksum, 20)) != 20) > + goto fail; These would cause mov_read_adrm() to fail but not neccessarily return an error code if any of these reads less. Is that intended ? thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Complexity theory is the science of finding the exact solution to an approximation. Benchmarking OTOH is finding an approximation of the exact
signature.asc
Description: PGP signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".