| Message ID | 20200827230609.2468-1-michael@niedermayer.cc |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [FFmpeg-devel] avcodec/cfhd: Check transform type | expand |
| Context | Check | Description |
|---|---|---|
| andriy/default | pending | |
| andriy/make | success | Make finished |
| andriy/make_fate | success | Make fate finished |
On 8/28/20, Michael Niedermayer <michael@niedermayer.cc> wrote: > Fixes: out of array access > Fixes: > 24823/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_CFHD_fuzzer-4855119863349248 > > 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 | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > It is not invalid, but unsupported. > diff --git a/libavcodec/cfhd.c b/libavcodec/cfhd.c > index 291d53e02e..be06b184de 100644 > --- a/libavcodec/cfhd.c > +++ b/libavcodec/cfhd.c > @@ -486,7 +486,7 @@ static int cfhd_decode(AVCodecContext *avctx, void > *data, int *got_frame, > s->sample_type = data; > av_log(avctx, AV_LOG_DEBUG, "Sample type? %"PRIu16"\n", data); > } else if (tag == TransformType) { > - if (data > 2) { > + if (data != 0 && data != 2) { > av_log(avctx, AV_LOG_ERROR, "Invalid transform type\n"); > ret = AVERROR(EINVAL); > break; > -- > 2.17.1 > > _______________________________________________ > 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 Fri, Aug 28, 2020 at 01:31:38AM +0200, Paul B Mahol wrote: > On 8/28/20, Michael Niedermayer <michael@niedermayer.cc> wrote: > > Fixes: out of array access > > Fixes: > > 24823/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_CFHD_fuzzer-4855119863349248 > > > > 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 | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > It is not invalid, but unsupported. fixed error code and message locally Is there some specification for this ? i was looking yesterday but google failed to point me to one thx [...]
On 8/28/20, Michael Niedermayer <michael@niedermayer.cc> wrote: > On Fri, Aug 28, 2020 at 01:31:38AM +0200, Paul B Mahol wrote: >> On 8/28/20, Michael Niedermayer <michael@niedermayer.cc> wrote: >> > Fixes: out of array access >> > Fixes: >> > 24823/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_CFHD_fuzzer-4855119863349248 >> > >> > 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 | 2 +- >> > 1 file changed, 1 insertion(+), 1 deletion(-) >> > >> >> It is not invalid, but unsupported. > > fixed error code and message locally > > Is there some specification for this ? > i was looking yesterday but google failed to point me to one > No specifications, just SDK on github. Also I'm unsure if that is sufficient fix for the underline issue. > thx > > [...] > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > Everything should be made as simple as possible, but not simpler. > -- Albert Einstein >
On Fri, Aug 28, 2020 at 10:24:04PM +0200, Paul B Mahol wrote: > On 8/28/20, Michael Niedermayer <michael@niedermayer.cc> wrote: > > On Fri, Aug 28, 2020 at 01:31:38AM +0200, Paul B Mahol wrote: > >> On 8/28/20, Michael Niedermayer <michael@niedermayer.cc> wrote: > >> > Fixes: out of array access > >> > Fixes: > >> > 24823/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_CFHD_fuzzer-4855119863349248 > >> > > >> > 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 | 2 +- > >> > 1 file changed, 1 insertion(+), 1 deletion(-) > >> > > >> > >> It is not invalid, but unsupported. > > > > fixed error code and message locally > > > > Is there some specification for this ? > > i was looking yesterday but google failed to point me to one > > > > No specifications, just SDK on github. > > Also I'm unsure if that is sufficient fix for the underline issue. I suspect the decoder has more issues. I was hoping that there is a specification that i could base validity and tag ordering checks on. Thanks [...]
On 8/28/20, Michael Niedermayer <michael@niedermayer.cc> wrote: > On Fri, Aug 28, 2020 at 10:24:04PM +0200, Paul B Mahol wrote: >> On 8/28/20, Michael Niedermayer <michael@niedermayer.cc> wrote: >> > On Fri, Aug 28, 2020 at 01:31:38AM +0200, Paul B Mahol wrote: >> >> On 8/28/20, Michael Niedermayer <michael@niedermayer.cc> wrote: >> >> > Fixes: out of array access >> >> > Fixes: >> >> > 24823/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_CFHD_fuzzer-4855119863349248 >> >> > >> >> > 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 | 2 +- >> >> > 1 file changed, 1 insertion(+), 1 deletion(-) >> >> > >> >> >> >> It is not invalid, but unsupported. >> > >> > fixed error code and message locally >> > >> > Is there some specification for this ? >> > i was looking yesterday but google failed to point me to one >> > >> >> No specifications, just SDK on github. >> >> Also I'm unsure if that is sufficient fix for the underline issue. > > I suspect the decoder has more issues. I was hoping that there is a > specification that i could base validity and tag ordering checks on. > Look at encoder, it follows tag order, note that some tags are purely optional. > > Thanks > > [...] > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > Many things microsoft did are stupid, but not doing something just because > microsoft did it is even more stupid. If everything ms did were stupid they > would be bankrupt already. >
Paul B Mahol wrote: >On 8/28/20, Michael Niedermayer <michael@niedermayer.cc> wrote: >>>>Is there some specification for this ? >>>>i was looking yesterday but google failed to point me to one >>>> >>> >>> No specifications, just SDK on github. >>> >>> Also I'm unsure if that is sufficient fix for the underline >>>issue. >> >>I suspect the decoder has more issues. I was hoping that there >>is a specification that i could base validity and tag ordering >>checks on. > >Look at encoder, it follows tag order, note that some tags are >purely optional. Behind their paywall there are: - SMPTE ST 2073-1:2014 - SMPTE RP 2073-2:2014 which do not include all the relevant information. The official SDK code by GoPro on GitHub is hard to read, at least for me. Emeric Grange did a cleanup and published an alternate code on his own repo. Best regards, Reto
diff --git a/libavcodec/cfhd.c b/libavcodec/cfhd.c index 291d53e02e..be06b184de 100644 --- a/libavcodec/cfhd.c +++ b/libavcodec/cfhd.c @@ -486,7 +486,7 @@ static int cfhd_decode(AVCodecContext *avctx, void *data, int *got_frame, s->sample_type = data; av_log(avctx, AV_LOG_DEBUG, "Sample type? %"PRIu16"\n", data); } else if (tag == TransformType) { - if (data > 2) { + if (data != 0 && data != 2) { av_log(avctx, AV_LOG_ERROR, "Invalid transform type\n"); ret = AVERROR(EINVAL); break;
Fixes: out of array access Fixes: 24823/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_CFHD_fuzzer-4855119863349248 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 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)