| Message ID | 20191125224717.26174-3-michael@niedermayer.cc |
|---|---|
| State | New |
| Headers | show |
Michael Niedermayer: > Fixes: memleak > Fixes: 19054/clusterfuzz-testcase-minimized-ffmpeg_DEMUXER_fuzzer-5673287506198528 > > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > --- > libavformat/vividas.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/libavformat/vividas.c b/libavformat/vividas.c > index f20af3d7c2..2c6a3daca7 100644 > --- a/libavformat/vividas.c > +++ b/libavformat/vividas.c > @@ -296,8 +296,10 @@ static int track_header(VividasDemuxContext *viv, AVFormatContext *s, uint8_t * > for (i=0;i<val_1;i++) { > int c = avio_r8(pb); > for (j=0;j<c;j++) { > - if (avio_feof(pb)) > + if (avio_feof(pb)) { > + av_free(pb); > return AVERROR_EOF; > + } > avio_r8(pb); // val_3 > avio_r8(pb); // val_4 > } > @@ -314,6 +316,7 @@ static int track_header(VividasDemuxContext *viv, AVFormatContext *s, uint8_t * > avio_seek(pb, off, SEEK_SET); > if (num_video != 1) { > av_log(s, AV_LOG_ERROR, "number of video tracks %d is not 1\n", num_video); > + av_free(pb); > return AVERROR_PATCHWELCOME; > } > A better solution for this is to not allocate the AVIOContext at all: Its lifetime does not extend beyond the function it is used in. I prepared a patch for this [1]. Given that vividas isn't covered by FATE (apart from the probe function) my patch is untested. But it should fix your fuzzing memleak. It will also imply that checking whether avformat_new_stream() failed as is done in [2] won't produce memleaks. - Andreas [1]: https://patchwork.ffmpeg.org/patch/16473/ [2]: https://ffmpeg.org/pipermail/ffmpeg-devel/2019-November/253551.html
diff --git a/libavformat/vividas.c b/libavformat/vividas.c index f20af3d7c2..2c6a3daca7 100644 --- a/libavformat/vividas.c +++ b/libavformat/vividas.c @@ -296,8 +296,10 @@ static int track_header(VividasDemuxContext *viv, AVFormatContext *s, uint8_t * for (i=0;i<val_1;i++) { int c = avio_r8(pb); for (j=0;j<c;j++) { - if (avio_feof(pb)) + if (avio_feof(pb)) { + av_free(pb); return AVERROR_EOF; + } avio_r8(pb); // val_3 avio_r8(pb); // val_4 } @@ -314,6 +316,7 @@ static int track_header(VividasDemuxContext *viv, AVFormatContext *s, uint8_t * avio_seek(pb, off, SEEK_SET); if (num_video != 1) { av_log(s, AV_LOG_ERROR, "number of video tracks %d is not 1\n", num_video); + av_free(pb); return AVERROR_PATCHWELCOME; }
Fixes: memleak Fixes: 19054/clusterfuzz-testcase-minimized-ffmpeg_DEMUXER_fuzzer-5673287506198528 Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> --- libavformat/vividas.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)