[FFmpeg-devel] [PATCH 3/3] avcodec/cfhd: More strictly check tag order and multiplicity
Paul B Mahol
onemda at gmail.com
Sun Dec 20 23:18:40 EET 2020
Unacceptable, please share privately sample that allows to reproduce this.
On Sun, Dec 20, 2020 at 10:16 PM Michael Niedermayer <michael at niedermayer.cc>
wrote:
> This is based on the encoder and a small number of CFHD sample files
> It should make the decoder more robust against crafted input.
> Due to the lack of a proper specification it is possible that this
> may be too strict and may need to be tuned as files not following this
> ordering are found.
>
> Fixes: segfault
> Fixes: OOM
> Fixes: null pointer dereference
> Fixes: left shift of negative value -12
> Fixes: out of array write
> Fixes:
> 25367/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_CFHD_fuzzer-4865603750592512
> Fixes:
> 25958/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_CFHD_fuzzer-4851579923202048
> Fixes:
> 25988/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_CFHD_fuzzer-5643617157513216
> Fixes:
> 25995/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_CFHD_fuzzer-5177442380283904
> Fixes:
> 25996/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_CFHD_fuzzer-5663296026574848
> Fixes:
> 26082/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_CFHD_fuzzer-5126180416782336
> Fixes:
> 27872/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_CFHD_fuzzer-4916296355151872
> Fixes:
> 28305/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_CFHD_fuzzer-6041755829010432
>
> Found-by: continuous fuzzing process
> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by
> <https://github.com/google/oss-fuzz/tree/master/projects/ffmpegSigned-off-by>:
> Michael Niedermayer <michael at niedermayer.cc>
> ---
> libavcodec/cfhd.c | 154 ++++++++++++++++++++++++++++++++++++++++++++++
> libavcodec/cfhd.h | 4 ++
> 2 files changed, 158 insertions(+)
>
> diff --git a/libavcodec/cfhd.c b/libavcodec/cfhd.c
> index 997beac7f4..5120e95f04 100644
> --- a/libavcodec/cfhd.c
> +++ b/libavcodec/cfhd.c
> @@ -363,6 +363,151 @@ static int alloc_buffers(AVCodecContext *avctx)
> return 0;
> }
>
> +typedef struct TagDescriptor {
> + int16_t previous_marker1;
> + int16_t previous_marker2;
> + uint8_t mandatory : 1;
> + uint8_t single : 1;
> +} TagDescriptor;
> +
> +static TagDescriptor tag_descriptor[LastTag]={
> + [SampleType /* 1*/] = { .previous_marker1 = 0x0c0c,
> .previous_marker2 = -1, },
> + [SampleIndexTable /* 2*/] = { .previous_marker1 = -1, .single =
> 1, .mandatory = 1 },
> + [ 3 ] = { .previous_marker1 = -1, .single =
> 1, .mandatory = 0 },
> + [BitstreamMarker /* 4*/] = { },
> + [VersionMajor /* 5*/] = { .previous_marker1 = -1, .single =
> 1, .mandatory = 0 },
> + [VersionMinor /* 6*/] = { .previous_marker1 = -1, .single =
> 1, .mandatory = 0 },
> + [VersionRevision /* 7*/] = { .previous_marker1 = -1, .single =
> 1, .mandatory = 0 },
> + [VersionEdit /* 8*/] = { .previous_marker1 = -1, .single =
> 1, .mandatory = 0 },
> + [ 9 ] = { .previous_marker1 = -1, .single =
> 1, .mandatory = 0 },
> + [TransformType /* 10*/] = { .previous_marker1 = -1, .single =
> 1, .mandatory = 1 },
> + [NumFrames /* 11*/] = { .previous_marker1 = -1, .single =
> 1, .mandatory = 1 },
> + [ChannelCount /* 12*/] = { .previous_marker1 = -1, .single =
> 1, .mandatory = 1 },
> + [WaveletCount /* 13*/] = { .previous_marker1 = -1, .single =
> 1, .mandatory = 1 },
> + [SubbandCount /* 14*/] = { .previous_marker1 = -1, .single =
> 1, .mandatory = 1 },
> + [NumSpatial /* 15*/] = { .previous_marker1 = -1, .single =
> 1, .mandatory = 1 },
> + [FirstWavelet /* 16*/] = { .previous_marker1 = -1, .single =
> 1, .mandatory = 1 },
> + [ 17 ] = { .previous_marker1 = -1, .single =
> 1, .mandatory = 0 },
> + [GroupTrailer /* 18*/] = { .previous_marker1 = 0x0c0c, .single =
> 1, .mandatory = 0 },
> + [FrameType /* 19*/] = { .previous_marker1 = -1, .single =
> 1, .mandatory = 0 },
> + [ImageWidth /* 20*/] = { .previous_marker1 = -1, .single =
> 1, .mandatory = 0 },
> + [ImageHeight /* 21*/] = { .previous_marker1 = -1, .single =
> 1, .mandatory = 0 },
> + [ 22 ] = { .previous_marker1 = -1, .single =
> 1, .mandatory = 0 },
> + [FrameIndex /* 23*/] = { .previous_marker1 = -1, .single =
> 1, .mandatory = 0 },
> + [ 24 ] = { .previous_marker1 = 0x0c0c, .single =
> 1, .mandatory = 0 },
> + [LowpassSubband /* 25*/] = { .previous_marker1 = 0x1a4a, .single =
> 1, .mandatory = 1 },
> + [NumLevels /* 26*/] = { .previous_marker1 = 0x1a4a, .single =
> 1, .mandatory = 1 },
> + [LowpassWidth /* 27*/] = { .previous_marker1 = 0x1a4a, .single =
> 1, .mandatory = 1 },
> + [LowpassHeight /* 28*/] = { .previous_marker1 = 0x1a4a, .single =
> 1, .mandatory = 1 },
> + [ 29 ] = { .previous_marker1 = 0x1a4a, .single =
> 1, .mandatory = 1 },
> + [ 30 ] = { .previous_marker1 = 0x1a4a, .single =
> 1, .mandatory = 1 },
> + [ 31 ] = { .previous_marker1 = 0x1a4a, .single =
> 1, .mandatory = 1 },
> + [ 32 ] = { .previous_marker1 = 0x1a4a, .single =
> 1, .mandatory = 1 },
> + [PixelOffset /* 33*/] = { .previous_marker1 = 0x1a4a, .single =
> 1, .mandatory = 1 },
> + [LowpassQuantization/*34*/] = { .previous_marker1 = 0x1a4a, .single =
> 1, .mandatory = 1 },
> + [LowpassPrecision /* 35*/] = { .previous_marker1 = 0x1a4a, .single =
> 1, .mandatory = 1 },
> + [ 36 ] = { .previous_marker1 = 0x1a4a, .single =
> 1, .mandatory = 0 },
> + [WaveletType /* 37*/] = { .previous_marker1 = 0x0d0d, .single =
> 1, .mandatory = 1 },
> + [WaveletNumber /* 38*/] = { .previous_marker1 = 0x0d0d, .single =
> 1, .mandatory = 1 },
> + [WaveletLevel /* 39*/] = { .previous_marker1 = 0x0d0d, .single =
> 1, .mandatory = 1 },
> + [NumBands /* 40*/] = { .previous_marker1 = 0x0d0d, .single =
> 1, .mandatory = 1 },
> + [HighpassWidth /* 41*/] = { .previous_marker1 = 0x0d0d, .single =
> 1, .mandatory = 1 },
> + [HighpassHeight /* 42*/] = { .previous_marker1 = 0x0d0d, .single =
> 1, .mandatory = 1 },
> + [LowpassBorder /* 43*/] = { .previous_marker1 = 0x0d0d, .single =
> 1, .mandatory = 1 },
> + [HighpassBorder /* 44*/] = { .previous_marker1 = 0x0d0d, .single =
> 1, .mandatory = 1 },
> + [LowpassScale /* 45*/] = { .previous_marker1 = 0x0d0d, .single =
> 1, .mandatory = 1 },
> + [LowpassDivisor /* 46*/] = { .previous_marker1 = 0x0d0d, .single =
> 1, .mandatory = 1 },
> + [ 47 ] = { .previous_marker1 = 0x0d0d, .single =
> 1, .mandatory = 0 },
> + [SubbandNumber /* 48*/] = { .previous_marker1 = 0x0e0e, .single =
> 1, .mandatory = 1 },
> + [BandWidth /* 49*/] = { .previous_marker1 = 0x0e0e, .single =
> 1, .mandatory = 1 },
> + [BandHeight /* 50*/] = { .previous_marker1 = 0x0e0e, .single =
> 1, .mandatory = 1 },
> + [SubbandBand /* 51*/] = { .previous_marker1 = 0x0e0e, .single =
> 1, .mandatory = 1 },
> + [BandEncoding /* 52*/] = { .previous_marker1 = 0x0e0e, .single =
> 1, .mandatory = 1 },
> + [Quantization /* 53*/] = { .previous_marker1 = 0x0e0e, .single =
> 1, .mandatory = 1 },
> + [BandScale /* 54*/] = { .previous_marker1 = 0x0e0e, .single =
> 1, .mandatory = 1 },
> + [BandHeader /* 55*/] = { .previous_marker1 = 0x0e0e, .single =
> 1, .mandatory = 1 },
> + [BandTrailer /* 56*/] = { .previous_marker1 = 0x0e0e, .single =
> 1, .mandatory = 1 },
> + [ChannelNumber /* 62*/] = { .previous_marker1 = 0x0c0c, .single =
> 1, .mandatory = 0 },
> + [ 63 ] = { .previous_marker1 = -1, .single =
> 1, .mandatory = 1 },
> + [ 64 ] = { .previous_marker1 = -1, .single =
> 1, .mandatory = 1 },
> + [ 65 ] = { .previous_marker1 = -1, .single =
> 1, .mandatory = 1 },
> + [ 66 ] = { .previous_marker1 = -1, .single =
> 1, .mandatory = 1 },
> + [SampleFlags /* 68*/] = { .previous_marker1 = -1, .single =
> 1, .mandatory = 0 },
> + [FrameNumber /* 69*/] = { .previous_marker1 = -1, .single =
> 1, .mandatory = 1 },
> + [Precision /* 70*/] = { .previous_marker1 = -1, .single =
> 1, .mandatory = 0 },
> + [InputFormat /* 71*/] = { .previous_marker1 = -1, .single =
> 1, .mandatory = 1 },
> + [BandCodingFlags /* 72*/] = { .previous_marker1 = 0x0e0e, .single =
> 1, .mandatory = 0 },
> + [ 73 ] = { .previous_marker1 = -1, .single =
> 1, .mandatory = 0 },
> + [PeakLevel /* 74*/] = { .previous_marker1 = 0x0e0e, .single =
> 1, .mandatory = 0 },
> + [PeakOffsetLow /* 75*/] = { .previous_marker1 = 0x0e0e, .single =
> 1, .mandatory = 0 },
> + [PeakOffsetHigh /* 76*/] = { .previous_marker1 = 0x0e0e, .single =
> 1, .mandatory = 0 },
> + [Version /* 79*/] = { .previous_marker1 = -1, .single =
> 1, .mandatory = 0 },
> + [ 80 ] = { .previous_marker1 = -1, .single =
> 1, .mandatory = 0 },
> + [ 81 ] = { .previous_marker1 = -1, .single =
> 1, .mandatory = 0 },
> + [BandSecondPass /* 82*/] = { },
> + [PrescaleTable /* 83*/] = { .previous_marker1 = -1, .single =
> 1, .mandatory = 0 },
> + [EncodedFormat /* 84*/] = { .previous_marker1 = -1, .single =
> 1, .mandatory = 0 },
> + [DisplayHeight /* 85*/] = { .previous_marker1 = -1, .single =
> 1, .mandatory = 0 },
> + [ 91 ] = { .previous_marker1 = -1, .single =
> 1, .mandatory = 0 },
> + [ 92 ] = { .previous_marker1 = -1, .single =
> 1, .mandatory = 0 },
> + [ 93 ] = { .previous_marker1 = -1, .single =
> 1, .mandatory = 0 },
> + [ChannelWidth /* 104*/] = { },
> + [ChannelHeight /* 105*/] = { },
> +};
> +
> +static int handle_tag_order(CFHDContext *s, int tag, int data)
> +{
> + TagDescriptor *desc;
> + int atag = abs(tag);
> + int i;
> +
> + // We do not restrict tags outside the enum
> + if (atag >= LastTag)
> + return 0;
> +
> + desc= &tag_descriptor[atag];
> + if (desc->single && s->tag_count[atag])
> + return AVERROR_INVALIDDATA;
> +
> + if (desc->previous_marker1 && s->previous_marker !=
> desc->previous_marker1) {
> + if (!desc->previous_marker2 || s->previous_marker !=
> desc->previous_marker2)
> + return AVERROR_INVALIDDATA;
> + } else if (tag == BitstreamMarker) {
> + if (s->previous_marker == -1 && data == 0x1a4a) {
> + ;
> + } else if (s->previous_marker == 0x1a4a && data == 0x0f0f) {
> + ;
> + } else if (s->previous_marker == 0x0f0f && data == 0x1b4b) {
> + ;
> + } else if (s->previous_marker == 0x1b4b && data == 0x0d0d) {
> + ;
> + } else if (s->previous_marker == 0x0d0d && data == 0x0e0e) {
> + ;
> + } else if (s->previous_marker == 0x0e0e && (data == 0x0c0c ||
> data == 0x0e0e)) {
> + ;
> + } else if (s->previous_marker == 0x0c0c && (data == 0x0d0d ||
> data == 0x1a4a)) {
> + ;
> + } else
> + return AVERROR_INVALIDDATA;
> +
> + for (i = 0; i<LastTag; i++) {
> + // Whenever we switch to a new marker we check the mandatory
> elements of the previous
> + if (tag_descriptor[i].previous_marker1 == s->previous_marker
> && tag_descriptor[i].mandatory && !s->tag_count[i]) {
> + return AVERROR_INVALIDDATA;
> + }
> +
> + if (tag_descriptor[i].previous_marker1 == data)
> + s->tag_count[i] = 0;
> + }
> + s->previous_marker = data;
> + } else if (!desc->previous_marker1)
> + return AVERROR_INVALIDDATA;
> +
> + s->tag_count[atag]++;
> +
> + return 0;
> +}
> +
> static int cfhd_decode(AVCodecContext *avctx, void *data, int *got_frame,
> AVPacket *avpkt)
> {
> @@ -376,6 +521,8 @@ static int cfhd_decode(AVCodecContext *avctx, void
> *data, int *got_frame,
>
> init_frame_defaults(s);
> s->planes = av_pix_fmt_count_planes(s->coded_format);
> + s->previous_marker = -1;
> + memset(s->tag_count, 0, sizeof(s->tag_count));
>
> bytestream2_init(&gb, avpkt->data, avpkt->size);
>
> @@ -387,6 +534,13 @@ static int cfhd_decode(AVCodecContext *avctx, void
> *data, int *got_frame,
> uint16_t abstag = abs(tag);
> int8_t abs_tag8 = abs(tag8);
> uint16_t data = bytestream2_get_be16(&gb);
> +
> + ret = handle_tag_order(s, tag, data);
> + if (ret < 0) {
> + av_log(avctx, AV_LOG_DEBUG, "Unexpected TAG %d data %X in
> %X\n", tag, data, s->previous_marker);
> + return ret;
> + }
> +
> if (abs_tag8 >= 0x60 && abs_tag8 <= 0x6f) {
> av_log(avctx, AV_LOG_DEBUG, "large len %x\n", ((tagu & 0xff)
> << 16) | data);
> } else if (tag == SampleFlags) {
> diff --git a/libavcodec/cfhd.h b/libavcodec/cfhd.h
> index 8ea91270cd..802c338f13 100644
> --- a/libavcodec/cfhd.h
> +++ b/libavcodec/cfhd.h
> @@ -93,6 +93,7 @@ enum CFHDParam {
> DisplayHeight = 85,
> ChannelWidth = 104,
> ChannelHeight = 105,
> + LastTag,
> };
>
> #define VLC_BITS 9
> @@ -184,6 +185,9 @@ typedef struct CFHDContext {
> Plane plane[4];
> Peak peak;
>
> + int previous_marker;
> + int tag_count[LastTag];
> +
> CFHDDSPContext dsp;
> } CFHDContext;
>
> --
> 2.17.1
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
More information about the ffmpeg-devel
mailing list