| Message ID | 20220719113453.23169-5-michael@niedermayer.cc |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,1/6] avcodec/mpeg4videoenc: fix encoding long frames | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/make_loongarch64 | success | Make finished |
| yinshiyou/make_fate_loongarch64 | success | Make fate finished |
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
On 7/19/2022 8:34 AM, Michael Niedermayer wrote: > Fixes: Timeout > Fixes: 49160/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_FFV1_fuzzer-5672826144686080 > > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > --- > libavcodec/ffv1dec.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/libavcodec/ffv1dec.c b/libavcodec/ffv1dec.c > index 01ddcaa512..9bdac0be4e 100644 > --- a/libavcodec/ffv1dec.c > +++ b/libavcodec/ffv1dec.c > @@ -883,7 +883,11 @@ static int decode_frame(AVCodecContext *avctx, AVFrame *rframe, > if (buf_size < avctx->width * avctx->height / (128*8)) > return AVERROR_INVALIDDATA; > } else { > - if (buf_size < avctx->height / 8) > + int i; for (int i... > + int w = avctx->width; > + for (i = 0; w > (1<<ff_log2_run[i]); i++) > + w -= ff_log2_run[i]; > + if (buf_size < (avctx->height + i + 6)/ 8) > return AVERROR_INVALIDDATA; > } >
On Tue, Jul 19, 2022 at 08:37:38AM -0300, James Almer wrote: > > > On 7/19/2022 8:34 AM, Michael Niedermayer wrote: > > Fixes: Timeout > > Fixes: 49160/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_FFV1_fuzzer-5672826144686080 > > > > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > > --- > > libavcodec/ffv1dec.c | 6 +++++- > > 1 file changed, 5 insertions(+), 1 deletion(-) > > > > diff --git a/libavcodec/ffv1dec.c b/libavcodec/ffv1dec.c > > index 01ddcaa512..9bdac0be4e 100644 > > --- a/libavcodec/ffv1dec.c > > +++ b/libavcodec/ffv1dec.c > > @@ -883,7 +883,11 @@ static int decode_frame(AVCodecContext *avctx, AVFrame *rframe, > > if (buf_size < avctx->width * avctx->height / (128*8)) > > return AVERROR_INVALIDDATA; > > } else { > > - if (buf_size < avctx->height / 8) > > + int i; > > for (int i... will apply with that change thx [...]
Michael Niedermayer: > On Tue, Jul 19, 2022 at 08:37:38AM -0300, James Almer wrote: >> >> >> On 7/19/2022 8:34 AM, Michael Niedermayer wrote: >>> Fixes: Timeout >>> Fixes: 49160/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_FFV1_fuzzer-5672826144686080 >>> >>> Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg >>> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> >>> --- >>> libavcodec/ffv1dec.c | 6 +++++- >>> 1 file changed, 5 insertions(+), 1 deletion(-) >>> >>> diff --git a/libavcodec/ffv1dec.c b/libavcodec/ffv1dec.c >>> index 01ddcaa512..9bdac0be4e 100644 >>> --- a/libavcodec/ffv1dec.c >>> +++ b/libavcodec/ffv1dec.c >>> @@ -883,7 +883,11 @@ static int decode_frame(AVCodecContext *avctx, AVFrame *rframe, >>> if (buf_size < avctx->width * avctx->height / (128*8)) >>> return AVERROR_INVALIDDATA; >>> } else { >>> - if (buf_size < avctx->height / 8) >>> + int i; >> >> for (int i... > > will apply with that change > > thx > James' suggestion made you use an uninitialized i in the actual check; and even the original check is wrong, as one can overrun ff_log2_run (unless there is a check that I am not missing). So it seems to me that reverting 15785e044ee1265464bb4f3ed727e2a8074f97b4 is appropriate. - Andreas
On Thu, Jul 21, 2022 at 12:17:22AM +0200, Andreas Rheinhardt wrote: > Michael Niedermayer: > > On Tue, Jul 19, 2022 at 08:37:38AM -0300, James Almer wrote: > >> > >> > >> On 7/19/2022 8:34 AM, Michael Niedermayer wrote: > >>> Fixes: Timeout > >>> Fixes: 49160/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_FFV1_fuzzer-5672826144686080 > >>> > >>> Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > >>> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > >>> --- > >>> libavcodec/ffv1dec.c | 6 +++++- > >>> 1 file changed, 5 insertions(+), 1 deletion(-) > >>> > >>> diff --git a/libavcodec/ffv1dec.c b/libavcodec/ffv1dec.c > >>> index 01ddcaa512..9bdac0be4e 100644 > >>> --- a/libavcodec/ffv1dec.c > >>> +++ b/libavcodec/ffv1dec.c > >>> @@ -883,7 +883,11 @@ static int decode_frame(AVCodecContext *avctx, AVFrame *rframe, > >>> if (buf_size < avctx->width * avctx->height / (128*8)) > >>> return AVERROR_INVALIDDATA; > >>> } else { > >>> - if (buf_size < avctx->height / 8) > >>> + int i; > >> > >> for (int i... > > > > will apply with that change > > > > thx > > > > James' suggestion made you use an uninitialized i in the actual check; yes > and even the original check is wrong, as one can overrun ff_log2_run > (unless there is a check that I am not missing). Theres a check but its too late > So it seems to me that > reverting 15785e044ee1265464bb4f3ed727e2a8074f97b4 is appropriate. not against that but heres a quick fix attempt Author: Michael Niedermayer <michael@niedermayer.cc> Date: Thu Jul 21 00:20:41 2022 +0200 avcodec/ffv1dec: Fix AC_GOLOMB_RICE min size check Found-by: mkver Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> diff --git a/libavcodec/ffv1dec.c b/libavcodec/ffv1dec.c index d71584505d..c6eca3227c 100644 --- a/libavcodec/ffv1dec.c +++ b/libavcodec/ffv1dec.c @@ -884,9 +884,14 @@ static int decode_frame(AVCodecContext *avctx, AVFrame *rframe, return AVERROR_INVALIDDATA; } else { int w = avctx->width; - for (int i = 0; w > (1<<ff_log2_run[i]); i++) + int s = 1 + w / (1<<23); + int i; + + w /= s; + + for (i = 0; w > (1<<ff_log2_run[i]); i++) w -= ff_log2_run[i]; - if (buf_size < (avctx->height + i + 6)/ 8) + if (buf_size < (avctx->height + s*i + 6)/ 8) return AVERROR_INVALIDDATA; } [...]
On Thu, Jul 21, 2022 at 12:46:38AM +0200, Michael Niedermayer wrote: > On Thu, Jul 21, 2022 at 12:17:22AM +0200, Andreas Rheinhardt wrote: > > Michael Niedermayer: > > > On Tue, Jul 19, 2022 at 08:37:38AM -0300, James Almer wrote: > > >> > > >> > > >> On 7/19/2022 8:34 AM, Michael Niedermayer wrote: > > >>> Fixes: Timeout > > >>> Fixes: 49160/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_FFV1_fuzzer-5672826144686080 > > >>> > > >>> Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > > >>> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > > >>> --- > > >>> libavcodec/ffv1dec.c | 6 +++++- > > >>> 1 file changed, 5 insertions(+), 1 deletion(-) > > >>> > > >>> diff --git a/libavcodec/ffv1dec.c b/libavcodec/ffv1dec.c > > >>> index 01ddcaa512..9bdac0be4e 100644 > > >>> --- a/libavcodec/ffv1dec.c > > >>> +++ b/libavcodec/ffv1dec.c > > >>> @@ -883,7 +883,11 @@ static int decode_frame(AVCodecContext *avctx, AVFrame *rframe, > > >>> if (buf_size < avctx->width * avctx->height / (128*8)) > > >>> return AVERROR_INVALIDDATA; > > >>> } else { > > >>> - if (buf_size < avctx->height / 8) > > >>> + int i; > > >> > > >> for (int i... > > > > > > will apply with that change > > > > > > thx > > > > > > > James' suggestion made you use an uninitialized i in the actual check; > > yes > > > > and even the original check is wrong, as one can overrun ff_log2_run > > (unless there is a check that I am not missing). > > Theres a check but its too late > > > > So it seems to me that > > reverting 15785e044ee1265464bb4f3ed727e2a8074f97b4 is appropriate. > > not against that but heres a quick fix attempt > > > Author: Michael Niedermayer <michael@niedermayer.cc> > Date: Thu Jul 21 00:20:41 2022 +0200 > > avcodec/ffv1dec: Fix AC_GOLOMB_RICE min size check will apply this so the uninitialized read is fixed. If this has some off by 1 error or something that can be adjusted later dont want to leave this bug open [...]
Michael Niedermayer: > On Thu, Jul 21, 2022 at 12:17:22AM +0200, Andreas Rheinhardt wrote: >> Michael Niedermayer: >>> On Tue, Jul 19, 2022 at 08:37:38AM -0300, James Almer wrote: >>>> >>>> >>>> On 7/19/2022 8:34 AM, Michael Niedermayer wrote: >>>>> Fixes: Timeout >>>>> Fixes: 49160/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_FFV1_fuzzer-5672826144686080 >>>>> >>>>> Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg >>>>> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> >>>>> --- >>>>> libavcodec/ffv1dec.c | 6 +++++- >>>>> 1 file changed, 5 insertions(+), 1 deletion(-) >>>>> >>>>> diff --git a/libavcodec/ffv1dec.c b/libavcodec/ffv1dec.c >>>>> index 01ddcaa512..9bdac0be4e 100644 >>>>> --- a/libavcodec/ffv1dec.c >>>>> +++ b/libavcodec/ffv1dec.c >>>>> @@ -883,7 +883,11 @@ static int decode_frame(AVCodecContext *avctx, AVFrame *rframe, >>>>> if (buf_size < avctx->width * avctx->height / (128*8)) >>>>> return AVERROR_INVALIDDATA; >>>>> } else { >>>>> - if (buf_size < avctx->height / 8) >>>>> + int i; >>>> >>>> for (int i... >>> >>> will apply with that change >>> >>> thx >>> >> >> James' suggestion made you use an uninitialized i in the actual check; > > yes > > >> and even the original check is wrong, as one can overrun ff_log2_run >> (unless there is a check that I am not missing). > > Theres a check but its too late > > >> So it seems to me that >> reverting 15785e044ee1265464bb4f3ed727e2a8074f97b4 is appropriate. > > not against that but heres a quick fix attempt > I thought that it would be easier to backport the fix if it were one patch; of course it was never my intention to force you to revert this. > > Author: Michael Niedermayer <michael@niedermayer.cc> > Date: Thu Jul 21 00:20:41 2022 +0200 > > avcodec/ffv1dec: Fix AC_GOLOMB_RICE min size check > > Found-by: mkver Please don't use my nickname in the future in commit messages. > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > > diff --git a/libavcodec/ffv1dec.c b/libavcodec/ffv1dec.c > index d71584505d..c6eca3227c 100644 > --- a/libavcodec/ffv1dec.c > +++ b/libavcodec/ffv1dec.c > @@ -884,9 +884,14 @@ static int decode_frame(AVCodecContext *avctx, AVFrame *rframe, > return AVERROR_INVALIDDATA; > } else { > int w = avctx->width; > - for (int i = 0; w > (1<<ff_log2_run[i]); i++) > + int s = 1 + w / (1<<23); > + int i; > + > + w /= s; > + > + for (i = 0; w > (1<<ff_log2_run[i]); i++) > w -= ff_log2_run[i]; > - if (buf_size < (avctx->height + i + 6)/ 8) > + if (buf_size < (avctx->height + s*i + 6)/ 8) > return AVERROR_INVALIDDATA; > } > > >
diff --git a/libavcodec/ffv1dec.c b/libavcodec/ffv1dec.c index 01ddcaa512..9bdac0be4e 100644 --- a/libavcodec/ffv1dec.c +++ b/libavcodec/ffv1dec.c @@ -883,7 +883,11 @@ static int decode_frame(AVCodecContext *avctx, AVFrame *rframe, if (buf_size < avctx->width * avctx->height / (128*8)) return AVERROR_INVALIDDATA; } else { - if (buf_size < avctx->height / 8) + int i; + int w = avctx->width; + for (i = 0; w > (1<<ff_log2_run[i]); i++) + w -= ff_log2_run[i]; + if (buf_size < (avctx->height + i + 6)/ 8) return AVERROR_INVALIDDATA; }
Fixes: Timeout Fixes: 49160/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_FFV1_fuzzer-5672826144686080 Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> --- libavcodec/ffv1dec.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)