From 1ba18717861c5da614619428389f29e00b175e8e Mon Sep 17 00:00:00 2001
From: Jingning Han <jingning@google.com>
Date: Thu, 12 Jun 2014 12:23:06 -0700
Subject: [PATCH] Fix out of boundary memory read in fuzz test on vpxdec

This commit fixes frame header decoding for superframe index, to
prevent out of boundary memory read triggered by fuzz test
vector. It resolves a chromium security violation issue
crbug.com/376802.

The issue was introduced in the change:

Add VPXD_SET_DECRYPTOR support to the VP9 decoder.
cl-id I88f86c8ff9af34e0b6531028b691921b54c2fc48

where the buffer was read before validation check on index offset
applied.

A test vector is added accordingly.

Change-Id: I41c988e776bbdd1033312a668e03a3dbcf44ca99
---
 test/test-data.sha1  |  3 ++-
 test/test.mk         |  2 ++
 test/test_vectors.cc |  3 ++-
 vp9/vp9_dx_iface.c   | 46 +++++++++++++++++++++++---------------------
 4 files changed, 30 insertions(+), 24 deletions(-)

diff --git a/test/test-data.sha1 b/test/test-data.sha1
index 9c2392974..0debef8af 100644
--- a/test/test-data.sha1
+++ b/test/test-data.sha1
@@ -639,4 +639,5 @@ 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 f0a27c70c..062ff4fa5 100644
--- a/test/test.mk
+++ b/test/test.mk
@@ -755,6 +755,8 @@ LIBVPX_TEST_DATA-$(CONFIG_VP9_DECODER) += vp90-2-15-segkey.webm
 LIBVPX_TEST_DATA-$(CONFIG_VP9_DECODER) += vp90-2-15-segkey.webm.md5
 LIBVPX_TEST_DATA-$(CONFIG_VP9_DECODER) += vp90-2-15-segkey_adpq.webm
 LIBVPX_TEST_DATA-$(CONFIG_VP9_DECODER) += vp90-2-15-segkey_adpq.webm.md5
+LIBVPX_TEST_DATA-$(CONFIG_VP9_DECODER) += vp90-2-15-fuzz-flicker.webm
+LIBVPX_TEST_DATA-$(CONFIG_VP9_DECODER) += vp90-2-15-fuzz-flicker.webm.md5
 
 ifeq ($(CONFIG_DECODE_PERF_TESTS),yes)
 # BBB VP9 streams
diff --git a/test/test_vectors.cc b/test/test_vectors.cc
index fd8c4c3eb..3873712cf 100644
--- a/test/test_vectors.cc
+++ b/test/test_vectors.cc
@@ -178,7 +178,8 @@ const char *const kVP9TestVectors[] = {
   "vp90-2-14-resize-fp-tiles-4-2.webm", "vp90-2-14-resize-fp-tiles-4-8.webm",
   "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-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 48110b414..3065acb9a 100644
--- a/vp9/vp9_dx_iface.c
+++ b/vp9/vp9_dx_iface.c
@@ -312,31 +312,33 @@ 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;
 
-    uint8_t marker2 = read_marker(decrypt_cb, decrypt_state,
-                                  data + data_sz - index_sz);
-
-    if (data_sz >= index_sz && marker2 == marker) {
-      // 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;
-      }
+    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;
+        }
 
-      for (i = 0; i < frames; i++) {
-        uint32_t this_sz = 0;
+        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;
-      }
+          for (j = 0; j < mag; ++j)
+            this_sz |= (*x++) << (j * 8);
+          sizes[i] = this_sz;
+        }
 
-      *count = frames;
+        *count = frames;
+      }
     }
   }
 }
-- 
GitLab