Message ID | 20191213002810.6440-4-michael@niedermayer.cc |
---|---|
State | Accepted |
Headers | show |
On Fri, 13. Dec 01:28, Michael Niedermayer wrote: > Fixes: Out of array access > Fixes: 19299/clusterfuzz-testcase-minimized-ffmpeg_BSF_HEVC_MP4TOANNEXB_fuzzer-5169193398042624 > > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > --- > libavcodec/hevc_mp4toannexb_bsf.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/libavcodec/hevc_mp4toannexb_bsf.c b/libavcodec/hevc_mp4toannexb_bsf.c > index d0f1b94f0e..baa93628ed 100644 > --- a/libavcodec/hevc_mp4toannexb_bsf.c > +++ b/libavcodec/hevc_mp4toannexb_bsf.c > @@ -152,8 +152,7 @@ static int hevc_mp4toannexb_filter(AVBSFContext *ctx, AVPacket *out) > extra_size = add_extradata * ctx->par_out->extradata_size; > got_irap |= is_irap; > > - if (SIZE_MAX - nalu_size < 4 || > - SIZE_MAX - 4 - nalu_size < extra_size) { > + if (FFMIN(INT_MAX, SIZE_MAX) < 4ULL + nalu_size + extra_size) { > ret = AVERROR_INVALIDDATA; > goto fail; > } > -- > 2.24.0 I already sent a patch for this: http://ffmpeg.org/pipermail/ffmpeg-devel/2019-December/253972.html
On Thu, Dec 12, 2019 at 07:36:50PM -0500, Andriy Gelman wrote: > On Fri, 13. Dec 01:28, Michael Niedermayer wrote: > > Fixes: Out of array access > > Fixes: 19299/clusterfuzz-testcase-minimized-ffmpeg_BSF_HEVC_MP4TOANNEXB_fuzzer-5169193398042624 > > > > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > > --- > > libavcodec/hevc_mp4toannexb_bsf.c | 3 +-- > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > diff --git a/libavcodec/hevc_mp4toannexb_bsf.c b/libavcodec/hevc_mp4toannexb_bsf.c > > index d0f1b94f0e..baa93628ed 100644 > > --- a/libavcodec/hevc_mp4toannexb_bsf.c > > +++ b/libavcodec/hevc_mp4toannexb_bsf.c > > @@ -152,8 +152,7 @@ static int hevc_mp4toannexb_filter(AVBSFContext *ctx, AVPacket *out) > > extra_size = add_extradata * ctx->par_out->extradata_size; > > got_irap |= is_irap; > > > > - if (SIZE_MAX - nalu_size < 4 || > > - SIZE_MAX - 4 - nalu_size < extra_size) { > > + if (FFMIN(INT_MAX, SIZE_MAX) < 4ULL + nalu_size + extra_size) { > > ret = AVERROR_INVALIDDATA; > > goto fail; > > } > > -- > > 2.24.0 > > I already sent a patch for this: > http://ffmpeg.org/pipermail/ffmpeg-devel/2019-December/253972.html yes but it depends on "distant" code behaving in specific ways. I prefer a single self contained check. Because its more robust to someone changing that distant code. So i would prefer to apply some "dumb" check. Do you agree ? Thanks [...]
On Fri, 13. Dec 18:51, Michael Niedermayer wrote: > On Thu, Dec 12, 2019 at 07:36:50PM -0500, Andriy Gelman wrote: > > On Fri, 13. Dec 01:28, Michael Niedermayer wrote: > > > Fixes: Out of array access > > > Fixes: 19299/clusterfuzz-testcase-minimized-ffmpeg_BSF_HEVC_MP4TOANNEXB_fuzzer-5169193398042624 > > > > > > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > > > --- > > > libavcodec/hevc_mp4toannexb_bsf.c | 3 +-- > > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > > > diff --git a/libavcodec/hevc_mp4toannexb_bsf.c b/libavcodec/hevc_mp4toannexb_bsf.c > > > index d0f1b94f0e..baa93628ed 100644 > > > --- a/libavcodec/hevc_mp4toannexb_bsf.c > > > +++ b/libavcodec/hevc_mp4toannexb_bsf.c > > > @@ -152,8 +152,7 @@ static int hevc_mp4toannexb_filter(AVBSFContext *ctx, AVPacket *out) > > > extra_size = add_extradata * ctx->par_out->extradata_size; > > > got_irap |= is_irap; > > > > > > - if (SIZE_MAX - nalu_size < 4 || > > > - SIZE_MAX - 4 - nalu_size < extra_size) { > > > + if (FFMIN(INT_MAX, SIZE_MAX) < 4ULL + nalu_size + extra_size) { > > > ret = AVERROR_INVALIDDATA; > > > goto fail; > > > } > > > -- > > > 2.24.0 > > > > I already sent a patch for this: > > http://ffmpeg.org/pipermail/ffmpeg-devel/2019-December/253972.html > > yes but it depends on "distant" code behaving in specific ways. I prefer > a single self contained check. Because its more robust to someone changing > that distant code. > So i would prefer to apply some "dumb" check. Do you agree ? I agree it's more robust. I'll keep this in mind for future Thanks,
On Fri, Dec 13, 2019 at 03:09:25PM -0500, Andriy Gelman wrote: > On Fri, 13. Dec 18:51, Michael Niedermayer wrote: > > On Thu, Dec 12, 2019 at 07:36:50PM -0500, Andriy Gelman wrote: > > > On Fri, 13. Dec 01:28, Michael Niedermayer wrote: > > > > Fixes: Out of array access > > > > Fixes: 19299/clusterfuzz-testcase-minimized-ffmpeg_BSF_HEVC_MP4TOANNEXB_fuzzer-5169193398042624 > > > > > > > > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > > > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > > > > --- > > > > libavcodec/hevc_mp4toannexb_bsf.c | 3 +-- > > > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > > > > > diff --git a/libavcodec/hevc_mp4toannexb_bsf.c b/libavcodec/hevc_mp4toannexb_bsf.c > > > > index d0f1b94f0e..baa93628ed 100644 > > > > --- a/libavcodec/hevc_mp4toannexb_bsf.c > > > > +++ b/libavcodec/hevc_mp4toannexb_bsf.c > > > > @@ -152,8 +152,7 @@ static int hevc_mp4toannexb_filter(AVBSFContext *ctx, AVPacket *out) > > > > extra_size = add_extradata * ctx->par_out->extradata_size; > > > > got_irap |= is_irap; > > > > > > > > - if (SIZE_MAX - nalu_size < 4 || > > > > - SIZE_MAX - 4 - nalu_size < extra_size) { > > > > + if (FFMIN(INT_MAX, SIZE_MAX) < 4ULL + nalu_size + extra_size) { > > > > ret = AVERROR_INVALIDDATA; > > > > goto fail; > > > > } > > > > -- > > > > 2.24.0 > > > > > > I already sent a patch for this: > > > http://ffmpeg.org/pipermail/ffmpeg-devel/2019-December/253972.html > > > > yes but it depends on "distant" code behaving in specific ways. I prefer > > a single self contained check. Because its more robust to someone changing > > that distant code. > > So i would prefer to apply some "dumb" check. Do you agree ? > > I agree it's more robust. I'll keep this in mind for future will apply thx [...]
diff --git a/libavcodec/hevc_mp4toannexb_bsf.c b/libavcodec/hevc_mp4toannexb_bsf.c index d0f1b94f0e..baa93628ed 100644 --- a/libavcodec/hevc_mp4toannexb_bsf.c +++ b/libavcodec/hevc_mp4toannexb_bsf.c @@ -152,8 +152,7 @@ static int hevc_mp4toannexb_filter(AVBSFContext *ctx, AVPacket *out) extra_size = add_extradata * ctx->par_out->extradata_size; got_irap |= is_irap; - if (SIZE_MAX - nalu_size < 4 || - SIZE_MAX - 4 - nalu_size < extra_size) { + if (FFMIN(INT_MAX, SIZE_MAX) < 4ULL + nalu_size + extra_size) { ret = AVERROR_INVALIDDATA; goto fail; }
Fixes: Out of array access Fixes: 19299/clusterfuzz-testcase-minimized-ffmpeg_BSF_HEVC_MP4TOANNEXB_fuzzer-5169193398042624 Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> --- libavcodec/hevc_mp4toannexb_bsf.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)