[FFmpeg-devel] [PATCH v4] avcodec/cfhd: More strictly check tag order and multiplicity

Michael Niedermayer michael at niedermayer.cc
Mon Aug 31 01:06:58 EEST 2020


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.

Signed-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 ea35f03869..6caaeb9c19 100644
--- a/libavcodec/cfhd.c
+++ b/libavcodec/cfhd.c
@@ -361,6 +361,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)
 {
@@ -374,6 +519,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);
 
@@ -385,6 +532,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



More information about the ffmpeg-devel mailing list