ref: aa1c813c43c6b2e43036d5573f361924195d65b7
parent: b3933e2d3ccf0de83441eac9d22a145c20ac6803
author: Vignesh Venkatasubramanian <[email protected]>
date: Wed Jun 22 06:24:27 EDT 2016
vp9: Fix potential SEGV in decoder_peek_si_internal decoder_peek_si_internal could potentially read more bytes than what actually exists in the input buffer. We check for the buffer size to be at least 8, but we try to read up to 10 bytes in the worst case. A well crafted file could thus cause a segfault. Likely change that introduced this bug was: https://chromium-review.googlesource.com/#/c/70439 (git hash: 7c43fb6) BUG=chromium:621095 Change-Id: Id74880cfdded44caaa45bbdbaac859c09d3db752
--- a/test/decode_api_test.cc
+++ b/test/decode_api_test.cc
@@ -146,6 +146,40 @@
TestVp9Controls(&dec);
EXPECT_EQ(VPX_CODEC_OK, vpx_codec_destroy(&dec));
}
+
+TEST(DecodeAPI, Vp9PeekSI) {+ const vpx_codec_iface_t *const codec = &vpx_codec_vp9_dx_algo;
+ // The first 9 bytes are valid and the rest of the bytes are made up. Until
+ // size 10, this should return VPX_CODEC_UNSUP_BITSTREAM and after that it
+ // should return VPX_CODEC_CORRUPT_FRAME.
+ const uint8_t data[32] = {+ 0x85, 0xa4, 0xc1, 0xa1, 0x38, 0x81, 0xa3, 0x49,
+ 0x83, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
+ 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
+ 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
+ };
+
+ for (uint32_t data_sz = 1; data_sz <= 32; ++data_sz) {+ // Verify behavior of vpx_codec_decode. vpx_codec_decode doesn't even get
+ // to decoder_peek_si_internal on frames of size < 8.
+ if (data_sz >= 8) {+ vpx_codec_ctx_t dec;
+ EXPECT_EQ(VPX_CODEC_OK, vpx_codec_dec_init(&dec, codec, NULL, 0));
+ EXPECT_EQ((data_sz < 10) ?
+ VPX_CODEC_UNSUP_BITSTREAM : VPX_CODEC_CORRUPT_FRAME,
+ vpx_codec_decode(&dec, data, data_sz, NULL, 0));
+ vpx_codec_iter_t iter = NULL;
+ EXPECT_EQ(NULL, vpx_codec_get_frame(&dec, &iter));
+ EXPECT_EQ(VPX_CODEC_OK, vpx_codec_destroy(&dec));
+ }
+
+ // Verify behavior of vpx_codec_peek_stream_info.
+ vpx_codec_stream_info_t si;
+ si.sz = sizeof(si);
+ EXPECT_EQ((data_sz < 10) ? VPX_CODEC_UNSUP_BITSTREAM : VPX_CODEC_OK,
+ vpx_codec_peek_stream_info(codec, data, data_sz, &si));
+ }
+}
#endif // CONFIG_VP9_DECODER
} // namespace
--- a/vp9/vp9_dx_iface.c
+++ b/vp9/vp9_dx_iface.c
@@ -127,7 +127,7 @@
vpx_decrypt_cb decrypt_cb,
void *decrypt_state) {int intra_only_flag = 0;
- uint8_t clear_buffer[9];
+ uint8_t clear_buffer[10];
if (data + data_sz <= data)
return VPX_CODEC_INVALID_PARAM;
@@ -141,6 +141,11 @@
data = clear_buffer;
}
+ // A maximum of 6 bits are needed to read the frame marker, profile and
+ // show_existing_frame.
+ if (data_sz < 1)
+ return VPX_CODEC_UNSUP_BITSTREAM;
+
{int show_frame;
int error_resilient;
@@ -154,15 +159,19 @@
if (profile >= MAX_PROFILES)
return VPX_CODEC_UNSUP_BITSTREAM;
- if ((profile >= 2 && data_sz <= 1) || data_sz < 1)
- return VPX_CODEC_UNSUP_BITSTREAM;
-
if (vpx_rb_read_bit(&rb)) { // show an existing frame+ // If profile is > 2 and show_existing_frame is true, then at least 1 more
+ // byte (6+3=9 bits) is needed.
+ if (profile > 2 && data_sz < 2)
+ return VPX_CODEC_UNSUP_BITSTREAM;
vpx_rb_read_literal(&rb, 3); // Frame buffer to show.
return VPX_CODEC_OK;
}
- if (data_sz <= 8)
+ // For the rest of the function, a maximum of 9 more bytes are needed
+ // (computed by taking the maximum possible bits needed in each case). Note
+ // that this has to be updated if we read any more bits in this function.
+ if (data_sz < 10)
return VPX_CODEC_UNSUP_BITSTREAM;
si->is_kf = !vpx_rb_read_bit(&rb);