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 |
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 |
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 [...]
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".
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 --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;
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(-)