From c3db2d8bc86b891f4a33dcb370d7172d1007cddc Mon Sep 17 00:00:00 2001 From: Jim Bankoski Date: Mon, 23 Jun 2014 07:04:57 -0700 Subject: [PATCH] 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 --- test/invalid_file_test.cc | 3 +- test/test-data.sha1 | 4 +-- test/test.mk | 2 ++ test/test_vectors.cc | 1 - vp9/vp9_dx_iface.c | 76 ++++++++++++++++++++++++--------------- 5 files changed, 54 insertions(+), 32 deletions(-) diff --git a/test/invalid_file_test.cc b/test/invalid_file_test.cc index e7f2a48da..6ec5564a1 100644 --- 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(sizeof(x) / sizeof(x[0])) diff --git a/test/test-data.sha1 b/test/test-data.sha1 index adfe15ec8..275583583 100644 --- 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 5d02d66da..7a3eaa9aa 100644 --- 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 75cd58f21..a6d546ea4 100644 --- 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 39807a1d6..a2969f4e3 100644 --- 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); + // 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; - if (marker == marker2) { - // Found a valid superframe index. - uint32_t i, j; - const uint8_t *x = &data[data_sz - index_sz + 1]; + { + const uint8_t marker2 = read_marker(decrypt_cb, decrypt_state, + data + data_sz - index_sz); - // 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 + // the matching marker byte at the front of the index therefore it's an + // invalid chunk. + if (marker != marker2) + return VPX_CODEC_CORRUPT_FRAME; + } - for (i = 0; i < frames; ++i) { - uint32_t this_sz = 0; + { + // Found a valid superframe index. + uint32_t i, j; + const uint8_t *x = &data[data_sz - index_sz + 1]; - for (j = 0; j < mag; ++j) - this_sz |= (*x++) << (j * 8); - sizes[i] = this_sz; - } - - *count = frames; + // 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