On Wed, Mar 25, 2020 at 02:39:19PM -0700, John Rummell wrote: > > > > These would cause mov_read_adrm() to fail but not neccessarily return an > > error code if any of these reads less. > > Is that intended ? > > > Not at all. Updated to always return AVERROR_INVALIDDATA. [...]
> mov.c | 34 ++++++++++++++++++++++++++-------- > oggdec.c | 3 ++- > wavdec.c | 12 ++++++++---- This should be split in 3 patches > 3 files changed, 36 insertions(+), 13 deletions(-) > 771646822442ae7d4c0e00b350fbcc872cb15ab9 > 0002-Check-return-value-from-avio_read-to-verify-data-act.patch > From 6751e6f594b0e0cba6fb0fbfdb7b0ab2c30c8512 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. [...] > @@ -1876,7 +1890,8 @@ static int mov_read_wave(MOVContext *c, AVIOContext > *pb, MOVAtom atom) > AV_WB32(st->codecpar->extradata , ALAC_EXTRADATA_SIZE); > AV_WB32(st->codecpar->extradata + 4, MKTAG('a','l','a','c')); > AV_WB64(st->codecpar->extradata + 12, buffer); > - avio_read(pb, st->codecpar->extradata + 20, 16); > + if (avio_read(pb, st->codecpar->extradata + 20, 16) != 16) > + return AVERROR_INVALIDDATA; > avio_skip(pb, atom.size - 24); > return 0; The commit message suggests that these fix uninitialized data This hare as well as some others work on 0 filled arrays. Its still good to check i think but for some of these cases the clearing becomes redundant if a check is added so the clearing should be removed then, also the commit message should be more clear that not all the changed cases fix uninitialized data > } > @@ -4376,7 +4391,8 @@ static int mov_read_keys(MOVContext *c, AVIOContext > *pb, MOVAtom atom) > c->meta_keys[i] = av_mallocz(key_size + 1); > if (!c->meta_keys[i]) > return AVERROR(ENOMEM); > - avio_read(pb, c->meta_keys[i], key_size); > + if (avio_read(pb, c->meta_keys[i], key_size) != key_size) > + return AVERROR_INVALIDDATA; > } > > return 0; This too is cleared [...] thx -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Frequently ignored answer#1 FFmpeg bugs should be sent to our bugtracker. User questions about the command line tools should be sent to the ffmpeg-user ML. And questions about how to use libav* should be sent to the libav-user ML.
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".