error check vp9 superframe parsing

This patch insures that the last byte of a chunk that contains a
valid superframe marker byte,  actually has a proper superframe index.
If not it returns an error.

As part of doing that the file : vp90-2-15-fuzz-flicker.webm now fails
to decode properly and moves to the invalid file test from the test
vector suite.

Change-Id: I5f1da7eb37282ec0c6394df5c73251a2df9c1744

diff --git a/test/invalid_file_test.cc b/test/invalid_file_test.cc
index e7f2a48..6ec5564 100644 (file)
--- a/test/invalid_file_test.cc
+++ b/test/invalid_file_test.cc
@@ -94,7 +94,8 @@ TEST_P(InvalidFileTest, ReturnCode) {
 }
 
 const char *const kVP9InvalidFileTests[] = {
-  "invalid-vp90-01.webm"
+  "invalid-vp90-01.webm",
+  "invalid-vp90-02.webm"
 };
 
 #define NELEMENTS(x) static_cast<int>(sizeof(x) / sizeof(x[0]))

diff --git a/test/test-data.sha1 b/test/test-data.sha1
index adfe15e..2755835 100644 (file)
--- a/test/test-data.sha1
+++ b/test/test-data.sha1
@@ -2,6 +2,8 @@ d5dfb0151c9051f8c85999255645d7a23916d3c0  hantro_collage_w352h288.yuv
 b87815bf86020c592ccc7a846ba2e28ec8043902  hantro_odd.yuv
 fe346136b9b8c1e6f6084cc106485706915795e4  invalid-vp90-01.webm
 25751f5d3b05ff03f0719ad42cd625348eb8961e  invalid-vp90-01.webm.res
+d78e2fceba5ac942246503ec8366f879c4775ca5  invalid-vp90-02.webm
+2dadee5306245fa5eeb0f99652d0e17afbcba96d  invalid-vp90-02.webm.res
 b1f1c3ec79114b9a0651af24ce634afb44a9a419  rush_hour_444.y4m
 5184c46ddca8b1fadd16742e8500115bc8f749da  vp80-00-comprehensive-001.ivf
 65bf1bbbced81b97bd030f376d1b7f61a224793f  vp80-00-comprehensive-002.ivf
@@ -642,5 +644,3 @@ e615575ded499ea1d992f3b38e3baa434509cdcd  vp90-2-15-segkey.webm
 e3ab35d4316c5e81325c50f5236ceca4bc0d35df  vp90-2-15-segkey.webm.md5
 9b7ca2cac09d34c4a5d296c1900f93b1e2f69d0d  vp90-2-15-segkey_adpq.webm
 8f46ba5f785d0c2170591a153e0d0d146a7c8090  vp90-2-15-segkey_adpq.webm.md5
-d78e2fceba5ac942246503ec8366f879c4775ca5  vp90-2-15-fuzz-flicker.webm
-bbd7dd15f43a703ff0a332fee4959e7b23bf77dc  vp90-2-15-fuzz-flicker.webm.md5

diff --git a/test/test.mk b/test/test.mk
index 5d02d66..7a3eaa9 100644 (file)
--- a/test/test.mk
+++ b/test/test.mk
@@ -763,6 +763,8 @@ LIBVPX_TEST_DATA-$(CONFIG_VP9_DECODER) += vp90-2-15-fuzz-flicker.webm.md5
 # Invalid files for testing libvpx error checking.
 LIBVPX_TEST_DATA-$(CONFIG_VP9_DECODER) += invalid-vp90-01.webm
 LIBVPX_TEST_DATA-$(CONFIG_VP9_DECODER) += invalid-vp90-01.webm.res
+LIBVPX_TEST_DATA-$(CONFIG_VP9_DECODER) += invalid-vp90-02.webm
+LIBVPX_TEST_DATA-$(CONFIG_VP9_DECODER) += invalid-vp90-02.webm.res
 
 ifeq ($(CONFIG_DECODE_PERF_TESTS),yes)
 # BBB VP9 streams

diff --git a/test/test_vectors.cc b/test/test_vectors.cc
index 75cd58f..a6d546e 100644 (file)
--- a/test/test_vectors.cc
+++ b/test/test_vectors.cc
@@ -180,7 +180,6 @@ const char *const kVP9TestVectors[] = {
   "vp90-2-14-resize-fp-tiles-8-16.webm", "vp90-2-14-resize-fp-tiles-8-1.webm",
   "vp90-2-14-resize-fp-tiles-8-2.webm", "vp90-2-14-resize-fp-tiles-8-4.webm",
   "vp90-2-15-segkey.webm", "vp90-2-15-segkey_adpq.webm",
-  "vp90-2-15-fuzz-flicker.webm"
 };
 const int kNumVP9TestVectors = NELEMENTS(kVP9TestVectors);
 #endif  // CONFIG_VP9_DECODER

diff --git a/vp9/vp9_dx_iface.c b/vp9/vp9_dx_iface.c
index 39807a1..a2969f4 100644 (file)
--- a/vp9/vp9_dx_iface.c
+++ b/vp9/vp9_dx_iface.c
@@ -382,10 +382,17 @@ static INLINE uint8_t read_marker(vpx_decrypt_cb decrypt_cb,
   return *data;
 }
 
-static void parse_superframe_index(const uint8_t *data, size_t data_sz,
-                                   uint32_t sizes[8], int *count,
-                                   vpx_decrypt_cb decrypt_cb,
-                                   void *decrypt_state) {
+static vpx_codec_err_t parse_superframe_index(const uint8_t *data,
+                                              size_t data_sz,
+                                              uint32_t sizes[8], int *count,
+                                              vpx_decrypt_cb decrypt_cb,
+                                              void *decrypt_state) {
+  // A chunk ending with a byte matching 0xc0 is an invalid chunk unless
+  // it is a super frame index. If the last byte of real video compression
+  // data is 0xc0 the encoder must add a 0 byte. If we have the marker but
+  // not the associated matching marker byte at the front of the index we have
+  // an invalid bitstream and need to return an error.
+
   uint8_t marker;
 
   assert(data_sz);
@@ -397,35 +404,46 @@ static void parse_superframe_index(const uint8_t *data, size_t data_sz,
     const uint32_t mag = ((marker >> 3) & 0x3) + 1;
     const size_t index_sz = 2 + mag * frames;
 
-    if (data_sz >= index_sz) {
-      uint8_t marker2 = read_marker(decrypt_cb, decrypt_state,
-                                    data + data_sz - index_sz);
-
-      if (marker == marker2) {
-        // Found a valid superframe index.
-        uint32_t i, j;
-        const uint8_t *x = &data[data_sz - index_sz + 1];
-
-        // Frames has a maximum of 8 and mag has a maximum of 4.
-        uint8_t clear_buffer[32];
-        assert(sizeof(clear_buffer) >= frames * mag);
-        if (decrypt_cb) {
-          decrypt_cb(decrypt_state, x, clear_buffer, frames * mag);
-          x = clear_buffer;
-        }
+    // This chunk is marked as having a superframe index but doesn't have
+    // enough data for it, thus it's an invalid superframe index.
+    if (data_sz < index_sz)
+      return VPX_CODEC_CORRUPT_FRAME;
 
-        for (i = 0; i < frames; ++i) {
-          uint32_t this_sz = 0;
+    {
+      const uint8_t marker2 = read_marker(decrypt_cb, decrypt_state,
+                                          data + data_sz - index_sz);
 
-          for (j = 0; j < mag; ++j)
-            this_sz |= (*x++) << (j * 8);
-          sizes[i] = this_sz;
-        }
+      // This chunk is marked as having a superframe index but doesn't have
+      // the matching marker byte at the front of the index therefore it's an
+      // invalid chunk.
+      if (marker != marker2)
+        return VPX_CODEC_CORRUPT_FRAME;
+    }
 
-        *count = frames;
+    {
+      // Found a valid superframe index.
+      uint32_t i, j;
+      const uint8_t *x = &data[data_sz - index_sz + 1];
+
+      // Frames has a maximum of 8 and mag has a maximum of 4.
+      uint8_t clear_buffer[32];
+      assert(sizeof(clear_buffer) >= frames * mag);
+      if (decrypt_cb) {
+        decrypt_cb(decrypt_state, x, clear_buffer, frames * mag);
+        x = clear_buffer;
       }
+
+      for (i = 0; i < frames; ++i) {
+        uint32_t this_sz = 0;
+
+        for (j = 0; j < mag; ++j)
+          this_sz |= (*x++) << (j * 8);
+        sizes[i] = this_sz;
+      }
+      *count = frames;
     }
   }
+  return VPX_CODEC_OK;
 }
 
 static vpx_codec_err_t decoder_decode(vpx_codec_alg_priv_t *ctx,
@@ -440,8 +458,10 @@ static vpx_codec_err_t decoder_decode(vpx_codec_alg_priv_t *ctx,
   if (data == NULL || data_sz == 0)
     return VPX_CODEC_INVALID_PARAM;
 
-  parse_superframe_index(data, data_sz, frame_sizes, &frame_count,
-                         ctx->decrypt_cb, ctx->decrypt_state);
+  res = parse_superframe_index(data, data_sz, frame_sizes, &frame_count,
+                               ctx->decrypt_cb, ctx->decrypt_state);
+  if (res != VPX_CODEC_OK)
+    return res;
 
   if (ctx->frame_parallel_decode) {
     // Decode in frame parallel mode. When decoding in this mode, the frame