diff mbox series

[FFmpeg-devel,1/3] avcodec/cfhd: Check transform_type consistently

Message ID 20210403143908.18494-1-michael@niedermayer.cc
State Accepted
Commit 20473a93d2a385a7137ef03e8b2604fa5e79eaa7
Headers show
Series [FFmpeg-devel,1/3] avcodec/cfhd: Check transform_type consistently | expand

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
Fixes: out of array accesses
Fixes: 29754/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_CFHD_fuzzer-6333598414274560
Fixes: 30519/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_CFHD_fuzzer-6298424511168512
Fixes: 30739/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_CFHD_fuzzer-5011292836462592

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 | 11 +++++++++--
 libavcodec/cfhd.h |  1 +
 2 files changed, 10 insertions(+), 2 deletions(-)

Comments

Michael Niedermayer April 3, 2021, 2:53 p.m. UTC | #1
On Sat, Apr 03, 2021 at 04:39:06PM +0200, Michael Niedermayer wrote:
> Fixes: out of array accesses
> Fixes: 29754/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_CFHD_fuzzer-6333598414274560
> Fixes: 30519/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_CFHD_fuzzer-6298424511168512
> Fixes: 30739/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_CFHD_fuzzer-5011292836462592
> 
> 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 | 11 +++++++++--
>  libavcodec/cfhd.h |  1 +
>  2 files changed, 10 insertions(+), 2 deletions(-)

I intend to apply this patchset soon. Also this patchset almost certainly
does not fix every issue in CFHD, so if someone is searching for code to
do a security review on. CFHD is likely an interresting candidate

thx

[...]
Paul B Mahol April 4, 2021, 7:46 a.m. UTC | #2
As always, make sure that you do not break or add new regressions for
decoding existing valid files.

On Sat, Apr 3, 2021 at 4:54 PM Michael Niedermayer <michael@niedermayer.cc>
wrote:

> On Sat, Apr 03, 2021 at 04:39:06PM +0200, Michael Niedermayer wrote:
> > Fixes: out of array accesses
> > Fixes:
> 29754/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_CFHD_fuzzer-6333598414274560
> > Fixes:
> 30519/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_CFHD_fuzzer-6298424511168512
> > Fixes:
> 30739/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_CFHD_fuzzer-5011292836462592
> >
> > 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 | 11 +++++++++--
> >  libavcodec/cfhd.h |  1 +
> >  2 files changed, 10 insertions(+), 2 deletions(-)
>
> I intend to apply this patchset soon. Also this patchset almost certainly
> does not fix every issue in CFHD, so if someone is searching for code to
> do a security review on. CFHD is likely an interresting candidate
>
> thx
>
> [...]
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> it is not once nor twice but times without number that the same ideas make
> their appearance in the world. -- Aristotle
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
James Almer April 4, 2021, 9:03 p.m. UTC | #3
On 4/4/2021 4:46 AM, Paul B Mahol wrote:
> As always, make sure that you do not break or add new regressions for
> decoding existing valid files.

Coverage of cfhd by FATE is really low. Pretty much half of the decoder 
is untested right now: 
http://coverage.ffmpeg.org/index.src_libavcodec_cfhd.c.html

Are there any samples with no copyright/license issues we could cut and 
add that make use of these untested code paths?

> 
> On Sat, Apr 3, 2021 at 4:54 PM Michael Niedermayer <michael@niedermayer.cc>
> wrote:
> 
>> On Sat, Apr 03, 2021 at 04:39:06PM +0200, Michael Niedermayer wrote:
>>> Fixes: out of array accesses
>>> Fixes:
>> 29754/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_CFHD_fuzzer-6333598414274560
>>> Fixes:
>> 30519/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_CFHD_fuzzer-6298424511168512
>>> Fixes:
>> 30739/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_CFHD_fuzzer-5011292836462592
>>>
>>> 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 | 11 +++++++++--
>>>   libavcodec/cfhd.h |  1 +
>>>   2 files changed, 10 insertions(+), 2 deletions(-)
>>
>> I intend to apply this patchset soon. Also this patchset almost certainly
>> does not fix every issue in CFHD, so if someone is searching for code to
>> do a security review on. CFHD is likely an interresting candidate
>>
>> thx
>>
>> [...]
>> --
>> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>>
>> it is not once nor twice but times without number that the same ideas make
>> their appearance in the world. -- Aristotle
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>> To unsubscribe, visit link above, or email
>> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>
diff mbox series

Patch

diff --git a/libavcodec/cfhd.c b/libavcodec/cfhd.c
index 1f2ee853c1..d719fbd65d 100644
--- a/libavcodec/cfhd.c
+++ b/libavcodec/cfhd.c
@@ -233,6 +233,7 @@  static void free_buffers(CFHDContext *s)
     }
     s->a_height = 0;
     s->a_width  = 0;
+    s->a_transform_type = INT_MIN;
 }
 
 static int alloc_buffers(AVCodecContext *avctx)
@@ -356,6 +357,7 @@  static int alloc_buffers(AVCodecContext *avctx)
         }
     }
 
+    s->a_transform_type = s->transform_type;
     s->a_height = s->coded_height;
     s->a_width  = s->coded_width;
     s->a_format = s->coded_format;
@@ -655,7 +657,8 @@  static int cfhd_decode(AVCodecContext *avctx, void *data, int *got_frame,
                 s->coded_height = s->a_height;
 
             if (s->a_width != s->coded_width || s->a_height != s->coded_height ||
-                s->a_format != s->coded_format) {
+                s->a_format != s->coded_format ||
+                s->transform_type != s->a_transform_type) {
                 free_buffers(s);
                 if ((ret = alloc_buffers(avctx)) < 0) {
                     free_buffers(s);
@@ -888,6 +891,7 @@  finish:
     ff_thread_finish_setup(avctx);
 
     if (!s->a_width || !s->a_height || s->a_format == AV_PIX_FMT_NONE ||
+        s->a_transform_type == INT_MIN ||
         s->coded_width || s->coded_height || s->coded_format != AV_PIX_FMT_NONE) {
         av_log(avctx, AV_LOG_ERROR, "Invalid dimensions\n");
         ret = AVERROR(EINVAL);
@@ -1381,12 +1385,14 @@  static int update_thread_context(AVCodecContext *dst, const AVCodecContext *src)
     if (pdst->plane[0].idwt_size != psrc->plane[0].idwt_size ||
         pdst->a_format != psrc->a_format ||
         pdst->a_width != psrc->a_width ||
-        pdst->a_height != psrc->a_height)
+        pdst->a_height != psrc->a_height ||
+        pdst->a_transform_type != psrc->a_transform_type)
         free_buffers(pdst);
 
     pdst->a_format = psrc->a_format;
     pdst->a_width  = psrc->a_width;
     pdst->a_height = psrc->a_height;
+    pdst->a_transform_type = psrc->a_transform_type;
     pdst->transform_type = psrc->transform_type;
     pdst->progressive = psrc->progressive;
     pdst->planes = psrc->planes;
@@ -1395,6 +1401,7 @@  static int update_thread_context(AVCodecContext *dst, const AVCodecContext *src)
         pdst->coded_width  = pdst->a_width;
         pdst->coded_height = pdst->a_height;
         pdst->coded_format = pdst->a_format;
+        pdst->transform_type = pdst->a_transform_type;
         ret = alloc_buffers(dst);
         if (ret < 0)
             return ret;
diff --git a/libavcodec/cfhd.h b/libavcodec/cfhd.h
index 8ea91270cd..4ec2a2ce8b 100644
--- a/libavcodec/cfhd.h
+++ b/libavcodec/cfhd.h
@@ -165,6 +165,7 @@  typedef struct CFHDContext {
     int a_width;
     int a_height;
     int a_format;
+    int a_transform_type;
 
     int bpc; // bits per channel/component
     int channel_cnt;