diff mbox series

[FFmpeg-devel,3/3] avcodec/cfhd: Keep track of which subbands have been read

Message ID 20210403143908.18494-3-michael@niedermayer.cc
State New
Headers show
Series [FFmpeg-devel,1/3] avcodec/cfhd: Check transform_type consistently
Related show

Checks

Context Check Description
andriy/x86_make success Make finished
andriy/x86_make_fate success Make fate finished
andriy/PPC64_make success Make finished
andriy/PPC64_make_fate success Make fate finished

Commit Message

Michael Niedermayer April 3, 2021, 2:39 p.m. UTC
This avoids use of uninitialized data
also several checks are inside the band reading code
so it is important that it is run at least once

Fixes: out of array accesses
Fixes: 28209/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_CFHD_fuzzer-5684714694377472
Fixes: 32124/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_CFHD_fuzzer-5425980681355264
Fixes: 30519/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_CFHD_fuzzer-4558757155700736

Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavcodec/cfhd.c | 20 ++++++++++++++++++++
 libavcodec/cfhd.h |  1 +
 2 files changed, 21 insertions(+)

Comments

Michael Niedermayer April 3, 2021, 4:30 p.m. UTC | #1
On Sat, Apr 03, 2021 at 04:39:08PM +0200, Michael Niedermayer wrote:
> This avoids use of uninitialized data
> also several checks are inside the band reading code
> so it is important that it is run at least once
> 
> Fixes: out of array accesses
> Fixes: 28209/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_CFHD_fuzzer-5684714694377472
> Fixes: 32124/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_CFHD_fuzzer-5425980681355264
> Fixes: 30519/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_CFHD_fuzzer-4558757155700736
> 
> Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>  libavcodec/cfhd.c | 20 ++++++++++++++++++++
>  libavcodec/cfhd.h |  1 +
>  2 files changed, 21 insertions(+)
> av_log(0,0, "TT %d ST %d\n", s->transform_type, s->sample_type);              s->codebook = 0;
> @@ -919,6 +929,16 @@ finish:
>          goto end;
>      }
>  
> +    for (plane = 0; plane < s->planes; plane++) {
> +        int o;
> +        for (o = 0; o < 4 ; o++) {
> +            if (!s->plane[plane].band[0][o].read_ok) {
> +                ret = AVERROR_INVALIDDATA;
> +                goto end;
> +            }
> +        }
> +    }

ive replaced this hunk by:
@@ -919,6 +929,22 @@ finish:
         goto end;
     }
 
+    for (plane = 0; plane < s->planes; plane++) {
+        int o, level;
+
+        for (level = 0; level < (s->transform_type == 0 ? DWT_LEVELS : DWT_LEVELS_3D) ; level++) {
+            if (s->transform_type == 2)
+                if (level == 2 || level == 5)
+                    continue;
+            for (o = !!level; o < 4 ; o++) {
+                if (!s->plane[plane].band[level][o].read_ok) {
+                    ret = AVERROR_INVALIDDATA;
+                    goto end;
+                }
+            }
+        }
+    }
+
     if (s->transform_type == 0 && s->sample_type != 1) {
         for (plane = 0; plane < s->planes && !ret; plane++) {
             /* level 1 */

so not just the first level is checked
             
[...]
diff mbox series

Patch

diff --git a/libavcodec/cfhd.c b/libavcodec/cfhd.c
index 8bf910cde6..61e02999e9 100644
--- a/libavcodec/cfhd.c
+++ b/libavcodec/cfhd.c
@@ -221,6 +221,7 @@  static void free_buffers(CFHDContext *s)
     int i, j;
 
     for (i = 0; i < FF_ARRAY_ELEMS(s->plane); i++) {
+        Plane *p = &s->plane[i];
         av_freep(&s->plane[i].idwt_buf);
         av_freep(&s->plane[i].idwt_tmp);
         s->plane[i].idwt_size = 0;
@@ -230,6 +231,12 @@  static void free_buffers(CFHDContext *s)
 
         for (j = 0; j < 10; j++)
             s->plane[i].l_h[j] = NULL;
+
+        for (j = 0; j < DWT_LEVELS_3D; j++)
+            p->band[j][0].read_ok =
+            p->band[j][1].read_ok =
+            p->band[j][2].read_ok =
+            p->band[j][3].read_ok = 0;
     }
     s->a_height = 0;
     s->a_width  = 0;
@@ -759,6 +766,8 @@  static int cfhd_decode(AVCodecContext *avctx, void *data, int *got_frame,
                        lowpass_width * sizeof(*coeff_data));
             }
 
+            s->plane[s->channel_num].band[0][0].read_ok = 1;
+
             av_log(avctx, AV_LOG_DEBUG, "Lowpass coefficients %d\n", lowpass_width * lowpass_height);
         }
 
@@ -891,6 +900,7 @@  static int cfhd_decode(AVCodecContext *avctx, void *data, int *got_frame,
                 bytestream2_seek(&gb, bytes, SEEK_CUR);
 
             av_log(avctx, AV_LOG_DEBUG, "End subband coeffs %i extra %i\n", count, count - expected);
+            s->plane[s->channel_num].band[s->level][s->subband_num].read_ok = 1;
 finish:
             if (s->subband_num_actual != 255)
                 s->codebook = 0;
@@ -919,6 +929,16 @@  finish:
         goto end;
     }
 
+    for (plane = 0; plane < s->planes; plane++) {
+        int o;
+        for (o = 0; o < 4 ; o++) {
+            if (!s->plane[plane].band[0][o].read_ok) {
+                ret = AVERROR_INVALIDDATA;
+                goto end;
+            }
+        }
+    }
+
     if (s->transform_type == 0 && s->sample_type != 1) {
         for (plane = 0; plane < s->planes && !ret; plane++) {
             /* level 1 */
diff --git a/libavcodec/cfhd.h b/libavcodec/cfhd.h
index 4ec2a2ce8b..19e5c7cf03 100644
--- a/libavcodec/cfhd.h
+++ b/libavcodec/cfhd.h
@@ -114,6 +114,7 @@  typedef struct SubBand {
     int width;
     int a_height;
     int height;
+    int8_t read_ok;
 } SubBand;
 
 typedef struct Plane {