diff mbox

[FFmpeg-devel,2/2] avcodec/cbs_vp9: Check data_size

Message ID 20191226001809.28204-2-michael@niedermayer.cc
State Accepted
Commit 4fa2d5a692f40c398a299acf2c6a20f5b98a3708
Headers show

Commit Message

Michael Niedermayer Dec. 26, 2019, 12:18 a.m. UTC
Fixes: out of array access
Fixes: 19542/clusterfuzz-testcase-minimized-ffmpeg_BSF_TRACE_HEADERS_fuzzer-5659498341728256

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_vp9.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

James Almer Dec. 26, 2019, 1:49 a.m. UTC | #1
On 12/25/2019 9:18 PM, Michael Niedermayer wrote:
> Fixes: out of array access
> Fixes: 19542/clusterfuzz-testcase-minimized-ffmpeg_BSF_TRACE_HEADERS_fuzzer-5659498341728256
> 
> 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_vp9.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/libavcodec/cbs_vp9.c b/libavcodec/cbs_vp9.c
> index 98730e03e3..ec82f11c76 100644
> --- a/libavcodec/cbs_vp9.c
> +++ b/libavcodec/cbs_vp9.c
> @@ -416,6 +416,9 @@ static int cbs_vp9_split_fragment(CodedBitstreamContext *ctx,
>      uint8_t superframe_header;
>      int err;
>  
> +    if (frag->data_size == 0)
> +        return AVERROR_INVALIDDATA;

cbs_h2645 and cbs_av1 return 0 when frag->data_size == 0, but cbs_jpeg
returns invalid data. I don't know which is more correct or ideal, but
if the failure doesn't happen here it will happen later in the process
when any code that finds out frag->nb_units is 0 will just bail out, so
LGTM.

> +
>      // Last byte in the packet.
>      superframe_header = frag->data[frag->data_size - 1];
>  
>
Michael Niedermayer Dec. 26, 2019, 2:56 p.m. UTC | #2
On Wed, Dec 25, 2019 at 10:49:47PM -0300, James Almer wrote:
> On 12/25/2019 9:18 PM, Michael Niedermayer wrote:
> > Fixes: out of array access
> > Fixes: 19542/clusterfuzz-testcase-minimized-ffmpeg_BSF_TRACE_HEADERS_fuzzer-5659498341728256
> > 
> > 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_vp9.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/libavcodec/cbs_vp9.c b/libavcodec/cbs_vp9.c
> > index 98730e03e3..ec82f11c76 100644
> > --- a/libavcodec/cbs_vp9.c
> > +++ b/libavcodec/cbs_vp9.c
> > @@ -416,6 +416,9 @@ static int cbs_vp9_split_fragment(CodedBitstreamContext *ctx,
> >      uint8_t superframe_header;
> >      int err;
> >  
> > +    if (frag->data_size == 0)
> > +        return AVERROR_INVALIDDATA;
> 
> cbs_h2645 and cbs_av1 return 0 when frag->data_size == 0, but cbs_jpeg
> returns invalid data. I don't know which is more correct or ideal, but
> if the failure doesn't happen here it will happen later in the process
> when any code that finds out frag->nb_units is 0 will just bail out, so
> LGTM.

will apply

thx

[...]
diff mbox

Patch

diff --git a/libavcodec/cbs_vp9.c b/libavcodec/cbs_vp9.c
index 98730e03e3..ec82f11c76 100644
--- a/libavcodec/cbs_vp9.c
+++ b/libavcodec/cbs_vp9.c
@@ -416,6 +416,9 @@  static int cbs_vp9_split_fragment(CodedBitstreamContext *ctx,
     uint8_t superframe_header;
     int err;
 
+    if (frag->data_size == 0)
+        return AVERROR_INVALIDDATA;
+
     // Last byte in the packet.
     superframe_header = frag->data[frag->data_size - 1];