generic-library/vpx
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

vp8_decode_frame: fix oob read on truncated key frame

Browse Source 
the check for error correction being disabled was overriding the data
length checks. this avoids returning incorrect information (width /
height) for the decoded frame which could result in inconsistent sizes
returned in to an application causing it to read beyond the bounds of
the frame allocation.

BUG=webm:1443
BUG=b/62458770

Change-Id: I063459674e01b57c0990cb29372e0eb9a1fbf342
This commit is contained in:
James Zern 2017-06-07 20:46:13 -07:00
parent 9cea3a3c4e
commit 45daecb4f7
4 changed files with 24 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
test/invalid_file_test.cc
View File

@ -120,6 +120,15 @@ class InvalidFileTest : public ::libvpx_test::DecoderTest,
TEST_P(InvalidFileTest, ReturnCode) { RunTest(); } TEST_P(InvalidFileTest, ReturnCode) { RunTest(); }
#if CONFIG_VP8_DECODER
const DecodeParam kVP8InvalidFileTests[] = {
{ 1, "invalid-bug-1443.ivf" },
};
VP8_INSTANTIATE_TEST_CASE(InvalidFileTest,
::testing::ValuesIn(kVP8InvalidFileTests));
#endif // CONFIG_VP8_DECODER
#if CONFIG_VP9_DECODER #if CONFIG_VP9_DECODER
const DecodeParam kVP9InvalidFileTests[] = { const DecodeParam kVP9InvalidFileTests[] = {
{ 1, "invalid-vp90-02-v2.webm" }, { 1, "invalid-vp90-02-v2.webm" },
@ -164,12 +173,12 @@ class InvalidFileInvalidPeekTest : public InvalidFileTest {
TEST_P(InvalidFileInvalidPeekTest, ReturnCode) { RunTest(); } TEST_P(InvalidFileInvalidPeekTest, ReturnCode) { RunTest(); }
#if CONFIG_VP8_DECODER #if CONFIG_VP8_DECODER
const DecodeParam kVP8InvalidFileTests[] = { const DecodeParam kVP8InvalidPeekTests[] = {
{ 1, "invalid-vp80-00-comprehensive-018.ivf.2kf_0x6.ivf" }, { 1, "invalid-vp80-00-comprehensive-018.ivf.2kf_0x6.ivf" },
}; };
VP8_INSTANTIATE_TEST_CASE(InvalidFileInvalidPeekTest, VP8_INSTANTIATE_TEST_CASE(InvalidFileInvalidPeekTest,
::testing::ValuesIn(kVP8InvalidFileTests)); ::testing::ValuesIn(kVP8InvalidPeekTests));
#endif // CONFIG_VP8_DECODER #endif // CONFIG_VP8_DECODER
#if CONFIG_VP9_DECODER #if CONFIG_VP9_DECODER

2
test/test-data.mk
View File

@ -732,6 +732,8 @@ LIBVPX_TEST_DATA-$(CONFIG_VP9_DECODER) += vp93-2-20-12bit-yuv444.webm.md5
endif # CONFIG_VP9_HIGHBITDEPTH endif # CONFIG_VP9_HIGHBITDEPTH
# Invalid files for testing libvpx error checking. # Invalid files for testing libvpx error checking.
LIBVPX_TEST_DATA-$(CONFIG_VP8_DECODER) += invalid-bug-1443.ivf
LIBVPX_TEST_DATA-$(CONFIG_VP8_DECODER) += invalid-bug-1443.ivf.res
LIBVPX_TEST_DATA-$(CONFIG_VP8_DECODER) += invalid-vp80-00-comprehensive-018.ivf.2kf_0x6.ivf LIBVPX_TEST_DATA-$(CONFIG_VP8_DECODER) += invalid-vp80-00-comprehensive-018.ivf.2kf_0x6.ivf
LIBVPX_TEST_DATA-$(CONFIG_VP8_DECODER) += invalid-vp80-00-comprehensive-018.ivf.2kf_0x6.ivf.res LIBVPX_TEST_DATA-$(CONFIG_VP8_DECODER) += invalid-vp80-00-comprehensive-018.ivf.2kf_0x6.ivf.res
LIBVPX_TEST_DATA-$(CONFIG_VP9_DECODER) += invalid-vp90-01-v3.webm LIBVPX_TEST_DATA-$(CONFIG_VP9_DECODER) += invalid-vp90-01-v3.webm

2
test/test-data.sha1
View File

@ -848,3 +848,5 @@ a000d568431d07379dd5a8ec066061c07e560b47 *invalid-vp90-2-00-quantizer-63.ivf.kf_
6fa3d3ac306a3d9ce1d610b78441dc00d2c2d4b9 *tos_vp8.webm 6fa3d3ac306a3d9ce1d610b78441dc00d2c2d4b9 *tos_vp8.webm
e402cbbf9e550ae017a1e9f1f73931c1d18474e8 *invalid-crbug-667044.webm e402cbbf9e550ae017a1e9f1f73931c1d18474e8 *invalid-crbug-667044.webm
d3964f9dad9f60363c81b688324d95b4ec7c8038 *invalid-crbug-667044.webm.res d3964f9dad9f60363c81b688324d95b4ec7c8038 *invalid-crbug-667044.webm.res
fd9df7f3f6992af1d7a9dde975c9a0d6f28c053d *invalid-bug-1443.ivf
fd3020fa6e9ca5966206738654c97dec313b0a95 *invalid-bug-1443.ivf.res

12
vp8/decoder/decodeframe.c
View File

@ -930,7 +930,7 @@ int vp8_decode_frame(VP8D_COMP *pbi) {
/* When error concealment is enabled we should only check the sync /* When error concealment is enabled we should only check the sync
* code if we have enough bits available * code if we have enough bits available
*/ */
if (!pbi->ec_active || data + 3 < data_end) { if (data + 3 < data_end) {
if (clear[0] != 0x9d || clear[1] != 0x01 || clear[2] != 0x2a) { if (clear[0] != 0x9d || clear[1] != 0x01 || clear[2] != 0x2a) {
vpx_internal_error(&pc->error, VPX_CODEC_UNSUP_BITSTREAM, vpx_internal_error(&pc->error, VPX_CODEC_UNSUP_BITSTREAM,
"Invalid frame sync code"); "Invalid frame sync code");
@ -941,13 +941,19 @@ int vp8_decode_frame(VP8D_COMP *pbi) {
* if we have enough data. Otherwise we will end up with the wrong * if we have enough data. Otherwise we will end up with the wrong
* size. * size.
*/ */
if (!pbi->ec_active || data + 6 < data_end) { if (data + 6 < data_end) {
pc->Width = (clear[3] | (clear[4] << 8)) & 0x3fff; pc->Width = (clear[3] | (clear[4] << 8)) & 0x3fff;
pc->horiz_scale = clear[4] >> 6; pc->horiz_scale = clear[4] >> 6;
pc->Height = (clear[5] | (clear[6] << 8)) & 0x3fff; pc->Height = (clear[5] | (clear[6] << 8)) & 0x3fff;
pc->vert_scale = clear[6] >> 6; pc->vert_scale = clear[6] >> 6;
}
data += 7; data += 7;
} else if (!pbi->ec_active) {
vpx_internal_error(&pc->error, VPX_CODEC_CORRUPT_FRAME,
"Truncated key frame header");
} else {
/* Error concealment is active, clear the frame. */
data = data_end;
}
} else { } else {
memcpy(&xd->pre, yv12_fb_new, sizeof(YV12_BUFFER_CONFIG)); memcpy(&xd->pre, yv12_fb_new, sizeof(YV12_BUFFER_CONFIG));
memcpy(&xd->dst, yv12_fb_new, sizeof(YV12_BUFFER_CONFIG)); memcpy(&xd->dst, yv12_fb_new, sizeof(YV12_BUFFER_CONFIG));