[FFmpeg-devel] [PATCH v2] avcodec/cfhd: More strictly check tag order

[Michael Niedermayer]

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 | 66 +++++++++++++++++++++++++++++++++++++++++++++++
 libavcodec/cfhd.h |  3 +++
 2 files changed, 69 insertions(+)

diff --git a/libavcodec/cfhd.c b/libavcodec/cfhd.c
index ea35f03869..c36edb9ef8 100644
--- a/libavcodec/cfhd.c
+++ b/libavcodec/cfhd.c
@@ -361,6 +361,64 @@ static int alloc_buffers(AVCodecContext *avctx)
     return 0;
 }
 
+static int handle_tag_order(CFHDContext *s, int tag, int data)
+{
+    int atag = abs(tag);
+    // We do not restrict tags outside the enum
+    if (atag >= LastTag)
+        return 0;
+
+    if (s->previous_marker == 0x88888888) {
+        if (tag >= 1 && tag <= 17 || tag >= 19 && tag <= 23 || atag >= 63 && atag <= 66 || atag >= 68 && atag <= 71 || atag == 73 || atag >= 79 && atag <= 81 || atag >= 83 && atag <= 85 || atag >= 91 && atag <= 93) {
+            ;
+        } else if (tag == BitstreamMarker && data == 0x1a4a) {
+            ;
+        } else
+            return AVERROR_INVALIDDATA;
+    } else if (s->previous_marker == 0x1a4a) {
+        if (tag >= 25 && tag <= 36) {
+            ;
+        } else if (tag == BitstreamMarker && data == 0xf0f) {
+            ;
+        } else
+            return AVERROR_INVALIDDATA;
+    } else if (s->previous_marker == 0xf0f) {
+        if (tag != BitstreamMarker || data != 0x1b4b)
+            return AVERROR_INVALIDDATA;
+    } else if (s->previous_marker == 0x1b4b) {
+        if (tag != BitstreamMarker || data != 0xd0d)
+            return AVERROR_INVALIDDATA;
+    } else if (s->previous_marker == 0xd0d) {
+        if (tag >= 37 && tag <= 47) {
+            ;
+        } else if (tag == BitstreamMarker && data == 0xe0e) {
+            ;
+        } else
+            return AVERROR_INVALIDDATA;
+    } else if (s->previous_marker == 0xe0e) {
+        if (tag >= 48 && tag <= 56 || tag == BandCodingFlags) {
+            ;
+        } else if (-tag >= 74 && -tag <= 76) {
+            ;
+        } else if (tag == BitstreamMarker && (data == 0xc0c || data == 0xe0e)) {
+            ;
+        } else
+            return AVERROR_INVALIDDATA;
+    } else if (s->previous_marker == 0xc0c) {
+        if (tag == 1 || tag == 18 || tag == 24 || tag == 62 ) {
+            ;
+        } else if (tag == BitstreamMarker && (data == 0xd0d || data == 0x1a4a)) {
+            ;
+        } else
+            return AVERROR_INVALIDDATA;
+    } else
+        return AVERROR_INVALIDDATA;
+
+    if (tag == BitstreamMarker)
+        s->previous_marker = data;
+    return 0;
+}
+
 static int cfhd_decode(AVCodecContext *avctx, void *data, int *got_frame,
                        AVPacket *avpkt)
 {
@@ -374,6 +432,7 @@ 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 = 0x88888888;
 
     bytestream2_init(&gb, avpkt->data, avpkt->size);
 
@@ -385,6 +444,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..c46ab01c17 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,8 @@ typedef struct CFHDContext {
     Plane plane[4];
     Peak peak;
 
+    int previous_marker;
+
     CFHDDSPContext dsp;
 } CFHDContext;
 
-- 
2.17.1