| Message ID | 20200307224043.12676-2-michael@niedermayer.cc |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,1/2] avcodec/cbs_jpeg: Check length for SOS | expand |
| Context | Check | Description |
|---|---|---|
| andriy/ffmpeg-patchwork | success | Make fate finished |
Michael Niedermayer: > Fixes: Assertion failure > Fixes: 19629/clusterfuzz-testcase-minimized-ffmpeg_BSF_H264_METADATA_fuzzer-5676822528524288 > > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > --- > libavcodec/cbs_h2645.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/libavcodec/cbs_h2645.c b/libavcodec/cbs_h2645.c > index c8347ba5fa..7427eefa30 100644 > --- a/libavcodec/cbs_h2645.c > +++ b/libavcodec/cbs_h2645.c > @@ -870,7 +870,8 @@ static int cbs_h264_read_nal_unit(CodedBitstreamContext *ctx, > "from slice data.\n", z); > len -= z; > } > - > + if (len <= pos / 8) > + return AVERROR_INVALIDDATA; > slice->data_size = len - pos / 8; > slice->data_ref = av_buffer_ref(unit->data_ref); > if (!slice->data_ref) > @@ -1052,7 +1053,8 @@ static int cbs_h265_read_nal_unit(CodedBitstreamContext *ctx, > "from slice data.\n", z); > len -= z; > } > - > + if (len <= pos / 8) > + return AVERROR_INVALIDDATA; > slice->data_size = len - pos / 8; > slice->data_ref = av_buffer_ref(unit->data_ref); > if (!slice->data_ref) > Imagine you have CAVLC encoded H.264 where there is nothing after the header except zero bits and where the slice header is not byte-aligned. Then av_assert0(temp) in cbs_h2645_write_slice_data() will be triggered both now as well as with your patch, because your check is too crude. I have already sent a patch [2] for this. It presupposes [1]. - Andreas [1]: https://patchwork.ffmpeg.org/project/ffmpeg/patch/20191209222604.28920-5-andreas.rheinhardt@gmail.com/ [2]: https://patchwork.ffmpeg.org/project/ffmpeg/patch/20191209222604.28920-6-andreas.rheinhardt@gmail.com/
On Mon, Mar 09, 2020 at 05:57:50AM +0100, Andreas Rheinhardt wrote: > Michael Niedermayer: > > Fixes: Assertion failure > > Fixes: 19629/clusterfuzz-testcase-minimized-ffmpeg_BSF_H264_METADATA_fuzzer-5676822528524288 > > > > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > > --- > > libavcodec/cbs_h2645.c | 6 ++++-- > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > diff --git a/libavcodec/cbs_h2645.c b/libavcodec/cbs_h2645.c > > index c8347ba5fa..7427eefa30 100644 > > --- a/libavcodec/cbs_h2645.c > > +++ b/libavcodec/cbs_h2645.c > > @@ -870,7 +870,8 @@ static int cbs_h264_read_nal_unit(CodedBitstreamContext *ctx, > > "from slice data.\n", z); > > len -= z; > > } > > - > > + if (len <= pos / 8) > > + return AVERROR_INVALIDDATA; > > slice->data_size = len - pos / 8; > > slice->data_ref = av_buffer_ref(unit->data_ref); > > if (!slice->data_ref) > > @@ -1052,7 +1053,8 @@ static int cbs_h265_read_nal_unit(CodedBitstreamContext *ctx, > > "from slice data.\n", z); > > len -= z; > > } > > - > > + if (len <= pos / 8) > > + return AVERROR_INVALIDDATA; > > slice->data_size = len - pos / 8; > > slice->data_ref = av_buffer_ref(unit->data_ref); > > if (!slice->data_ref) > > > > Imagine you have CAVLC encoded H.264 where there is nothing after the > header except zero bits and where the slice header is not > byte-aligned. Then av_assert0(temp) in cbs_h2645_write_slice_data() > will be triggered both now as well as with your patch, because your > check is too crude. > > I have already sent a patch [2] for this. It presupposes [1]. the assert you speak about wasnt the one failing for this. but your patches are a nicer solution so ill apply them Thanks [...]
diff --git a/libavcodec/cbs_h2645.c b/libavcodec/cbs_h2645.c index c8347ba5fa..7427eefa30 100644 --- a/libavcodec/cbs_h2645.c +++ b/libavcodec/cbs_h2645.c @@ -870,7 +870,8 @@ static int cbs_h264_read_nal_unit(CodedBitstreamContext *ctx, "from slice data.\n", z); len -= z; } - + if (len <= pos / 8) + return AVERROR_INVALIDDATA; slice->data_size = len - pos / 8; slice->data_ref = av_buffer_ref(unit->data_ref); if (!slice->data_ref) @@ -1052,7 +1053,8 @@ static int cbs_h265_read_nal_unit(CodedBitstreamContext *ctx, "from slice data.\n", z); len -= z; } - + if (len <= pos / 8) + return AVERROR_INVALIDDATA; slice->data_size = len - pos / 8; slice->data_ref = av_buffer_ref(unit->data_ref); if (!slice->data_ref)
Fixes: Assertion failure Fixes: 19629/clusterfuzz-testcase-minimized-ffmpeg_BSF_H264_METADATA_fuzzer-5676822528524288 Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> --- libavcodec/cbs_h2645.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)