Message ID | 20220204005811.5459-1-michael@niedermayer.cc |
---|---|
State | New |
Headers | show |
Series | [FFmpeg-devel,1/2] avformat/matroskadec: Fix infinite loop with bz decompression | expand |
Context | Check | Description |
---|---|---|
andriy/make_x86 | success | Make finished |
andriy/make_fate_x86 | success | Make fate finished |
andriy/make_ppc | success | Make finished |
andriy/make_fate_ppc | success | Make fate finished |
Michael Niedermayer: > Fixes: Infinite loop > Fixes: 43932/clusterfuzz-testcase-minimized-ffmpeg_dem_MATROSKA_fuzzer-6175167573786624 > > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > --- > libavformat/matroskadec.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c > index d165f6ab90..5a9acfb247 100644 > --- a/libavformat/matroskadec.c > +++ b/libavformat/matroskadec.c > @@ -1742,7 +1742,7 @@ static int matroska_decode_buffer(uint8_t **buf, int *buf_size, > case MATROSKA_TRACK_ENCODING_COMP_BZLIB: > { > bz_stream bzstream = { 0 }; > - if (BZ2_bzDecompressInit(&bzstream, 0, 0) != BZ_OK) > + if (!pkt_size || BZ2_bzDecompressInit(&bzstream, 0, 0) != BZ_OK) > return -1; > bzstream.next_in = data; > bzstream.avail_in = isize; I see nothing in the zlib-API manual that would preclude this from happening with zlib, too, so it should be checked there, too. LGTM apart from that. - Andreas
On Fri, Feb 04, 2022 at 04:29:18AM +0100, Andreas Rheinhardt wrote: > Michael Niedermayer: > > Fixes: Infinite loop > > Fixes: 43932/clusterfuzz-testcase-minimized-ffmpeg_dem_MATROSKA_fuzzer-6175167573786624 > > > > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > > --- > > libavformat/matroskadec.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c > > index d165f6ab90..5a9acfb247 100644 > > --- a/libavformat/matroskadec.c > > +++ b/libavformat/matroskadec.c > > @@ -1742,7 +1742,7 @@ static int matroska_decode_buffer(uint8_t **buf, int *buf_size, > > case MATROSKA_TRACK_ENCODING_COMP_BZLIB: > > { > > bz_stream bzstream = { 0 }; > > - if (BZ2_bzDecompressInit(&bzstream, 0, 0) != BZ_OK) > > + if (!pkt_size || BZ2_bzDecompressInit(&bzstream, 0, 0) != BZ_OK) > > return -1; > > bzstream.next_in = data; > > bzstream.avail_in = isize; > > I see nothing in the zlib-API manual that would preclude this from > happening with zlib, too, so it should be checked there, too. > LGTM apart from that. It didnt happen with the same case when i just routed it to the zlib instead of bz case. But i didnt look what happened exactly [...]
On Fri, Feb 04, 2022 at 04:07:10PM +0100, Michael Niedermayer wrote: > On Fri, Feb 04, 2022 at 04:29:18AM +0100, Andreas Rheinhardt wrote: > > Michael Niedermayer: > > > Fixes: Infinite loop > > > Fixes: 43932/clusterfuzz-testcase-minimized-ffmpeg_dem_MATROSKA_fuzzer-6175167573786624 > > > > > > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > > > --- > > > libavformat/matroskadec.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c > > > index d165f6ab90..5a9acfb247 100644 > > > --- a/libavformat/matroskadec.c > > > +++ b/libavformat/matroskadec.c > > > @@ -1742,7 +1742,7 @@ static int matroska_decode_buffer(uint8_t **buf, int *buf_size, > > > case MATROSKA_TRACK_ENCODING_COMP_BZLIB: > > > { > > > bz_stream bzstream = { 0 }; > > > - if (BZ2_bzDecompressInit(&bzstream, 0, 0) != BZ_OK) > > > + if (!pkt_size || BZ2_bzDecompressInit(&bzstream, 0, 0) != BZ_OK) > > > return -1; > > > bzstream.next_in = data; > > > bzstream.avail_in = isize; > > > > I see nothing in the zlib-API manual that would preclude this from > > happening with zlib, too, so it should be checked there, too. > > LGTM apart from that. > > It didnt happen with the same case when i just routed it to the zlib > instead of bz case. But i didnt look what happened exactly will apply with the check for zlib too, it feels like a good idea even when it seems not needed thx [...]
diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c index d165f6ab90..5a9acfb247 100644 --- a/libavformat/matroskadec.c +++ b/libavformat/matroskadec.c @@ -1742,7 +1742,7 @@ static int matroska_decode_buffer(uint8_t **buf, int *buf_size, case MATROSKA_TRACK_ENCODING_COMP_BZLIB: { bz_stream bzstream = { 0 }; - if (BZ2_bzDecompressInit(&bzstream, 0, 0) != BZ_OK) + if (!pkt_size || BZ2_bzDecompressInit(&bzstream, 0, 0) != BZ_OK) return -1; bzstream.next_in = data; bzstream.avail_in = isize;
Fixes: Infinite loop Fixes: 43932/clusterfuzz-testcase-minimized-ffmpeg_dem_MATROSKA_fuzzer-6175167573786624 Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> --- libavformat/matroskadec.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)