diff mbox series

[FFmpeg-devel,2/2] avcodec/hevc_ps: Dont leave invalid cpb_cnt_minus1 in the context

Message ID 20231018004935.18309-2-michael@niedermayer.cc
State New
Headers show
Series [FFmpeg-devel,1/2] avformat/mxfdec: Check klv offset | expand

Checks

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

Commit Message

Michael Niedermayer Oct. 18, 2023, 12:49 a.m. UTC
Fixes: index 32 out of bounds for type 'uint32_t [32]'
Fixes: 63003/clusterfuzz-testcase-minimized-ffmpeg_DEMUXER_fuzzer-4685160840560640

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_ps.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Andreas Rheinhardt Oct. 18, 2023, 1:17 p.m. UTC | #1
Michael Niedermayer:
> Fixes: index 32 out of bounds for type 'uint32_t [32]'
> Fixes: 63003/clusterfuzz-testcase-minimized-ffmpeg_DEMUXER_fuzzer-4685160840560640
> 
> 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_ps.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/libavcodec/hevc_ps.c b/libavcodec/hevc_ps.c
> index a6b64b92e3..f4365ef5b5 100644
> --- a/libavcodec/hevc_ps.c
> +++ b/libavcodec/hevc_ps.c
> @@ -421,6 +421,7 @@ static int decode_hrd(GetBitContext *gb, int common_inf_present,
>              if (hdr->cpb_cnt_minus1[i] > 31) {
>                  av_log(NULL, AV_LOG_ERROR, "nb_cpb %d invalid\n",
>                         hdr->cpb_cnt_minus1[i]);
> +                hdr->cpb_cnt_minus1[i] = 0;
>                  return AVERROR_INVALIDDATA;
>              }
>          }

There is a second issue here: There can be truncation during the
previous assignment, because cpb_cnt_minus1 is uint8_t. So this should
be fixed by properly checking the value and only putting it in the
parameter set after it has been validated (which also avoids having to
reset it).

- Andreas
Michael Niedermayer Oct. 19, 2023, 4:55 p.m. UTC | #2
On Wed, Oct 18, 2023 at 03:17:41PM +0200, Andreas Rheinhardt wrote:
> Michael Niedermayer:
> > Fixes: index 32 out of bounds for type 'uint32_t [32]'
> > Fixes: 63003/clusterfuzz-testcase-minimized-ffmpeg_DEMUXER_fuzzer-4685160840560640
> > 
> > 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_ps.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/libavcodec/hevc_ps.c b/libavcodec/hevc_ps.c
> > index a6b64b92e3..f4365ef5b5 100644
> > --- a/libavcodec/hevc_ps.c
> > +++ b/libavcodec/hevc_ps.c
> > @@ -421,6 +421,7 @@ static int decode_hrd(GetBitContext *gb, int common_inf_present,
> >              if (hdr->cpb_cnt_minus1[i] > 31) {
> >                  av_log(NULL, AV_LOG_ERROR, "nb_cpb %d invalid\n",
> >                         hdr->cpb_cnt_minus1[i]);
> > +                hdr->cpb_cnt_minus1[i] = 0;
> >                  return AVERROR_INVALIDDATA;
> >              }
> >          }
> 
> There is a second issue here: There can be truncation during the
> previous assignment, because cpb_cnt_minus1 is uint8_t. So this should
> be fixed by properly checking the value and only putting it in the
> parameter set after it has been validated (which also avoids having to
> reset it).

ok, will apply with that

thx

[...]
diff mbox series

Patch

diff --git a/libavcodec/hevc_ps.c b/libavcodec/hevc_ps.c
index a6b64b92e3..f4365ef5b5 100644
--- a/libavcodec/hevc_ps.c
+++ b/libavcodec/hevc_ps.c
@@ -421,6 +421,7 @@  static int decode_hrd(GetBitContext *gb, int common_inf_present,
             if (hdr->cpb_cnt_minus1[i] > 31) {
                 av_log(NULL, AV_LOG_ERROR, "nb_cpb %d invalid\n",
                        hdr->cpb_cnt_minus1[i]);
+                hdr->cpb_cnt_minus1[i] = 0;
                 return AVERROR_INVALIDDATA;
             }
         }