From f242a4a1a1532c1e3a06f0aff35679bbebb1240b Mon Sep 17 00:00:00 2001 From: Frank Barchard Date: Thu, 30 Jul 2015 15:49:48 -0700 Subject: [PATCH] ValidateJpeg check for valid pointer and size R=harryjin@google.com BUG=chromium:497297 Review URL: https://webrtc-codereview.appspot.com/57649004. --- README.chromium | 2 +- include/libyuv/version.h | 2 +- source/mjpeg_validate.cc | 80 ++++++++++----------------------------- unit_test/convert_test.cc | 57 +++++++++++++++++++++------- 4 files changed, 66 insertions(+), 75 deletions(-) diff --git a/README.chromium b/README.chromium index edb7feac7..95a2f75db 100644 --- a/README.chromium +++ b/README.chromium @@ -1,6 +1,6 @@ Name: libyuv URL: http://code.google.com/p/libyuv/ -Version: 1454 +Version: 1455 License: BSD License File: LICENSE diff --git a/include/libyuv/version.h b/include/libyuv/version.h index 571bc0425..4bf3f8c38 100644 --- a/include/libyuv/version.h +++ b/include/libyuv/version.h @@ -11,6 +11,6 @@ #ifndef INCLUDE_LIBYUV_VERSION_H_ // NOLINT #define INCLUDE_LIBYUV_VERSION_H_ -#define LIBYUV_VERSION 1454 +#define LIBYUV_VERSION 1455 #endif // INCLUDE_LIBYUV_VERSION_H_ NOLINT diff --git a/source/mjpeg_validate.cc b/source/mjpeg_validate.cc index 8edfbe1e7..fdb8d6ced 100644 --- a/source/mjpeg_validate.cc +++ b/source/mjpeg_validate.cc @@ -17,51 +17,22 @@ namespace libyuv { extern "C" { #endif -// Enable this to try scasb implementation. -// #define ENABLE_SCASB 1 - -#ifdef ENABLE_SCASB - -// Multiple of 1. -__declspec(naked) -const uint8* ScanRow_ERMS(const uint8* src, uint32 val, int count) { - __asm { - mov edx, edi - mov edi, [esp + 4] // src - mov eax, [esp + 8] // val - mov ecx, [esp + 12] // count - repne scasb - jne sr99 - mov eax, edi - sub eax, 1 - mov edi, edx - ret - - sr99: - mov eax, 0 - mov edi, edx - ret - } -} -#endif - -// Helper function to scan for EOI marker. +// Helper function to scan for EOI marker (0xff 0xd9). static LIBYUV_BOOL ScanEOI(const uint8* sample, size_t sample_size) { - const uint8* end = sample + sample_size - 1; - const uint8* it = sample; - for (;;) { -#ifdef ENABLE_SCASB - it = ScanRow_ERMS(it, 0xff, end - it); -#else - it = static_cast(memchr(it, 0xff, end - it)); -#endif - if (it == NULL) { - break; + if (sample_size >= 2) { + const uint8* end = sample + sample_size - 1; + const uint8* it = sample; + while (it < end) { + // TODO(fbarchard): scan for 0xd9 instead. + it = static_cast(memchr(it, 0xff, end - it)); + if (it == NULL) { + break; + } + if (it[1] == 0xd9) { + return LIBYUV_TRUE; // Success: Valid jpeg. + } + ++it; // Skip over current 0xff. } - if (it[1] == 0xd9) { - return LIBYUV_TRUE; // Success: Valid jpeg. - } - ++it; // Skip over current 0xff. } // ERROR: Invalid jpeg end code not found. Size sample_size return LIBYUV_FALSE; @@ -69,29 +40,18 @@ static LIBYUV_BOOL ScanEOI(const uint8* sample, size_t sample_size) { // Helper function to validate the jpeg appears intact. LIBYUV_BOOL ValidateJpeg(const uint8* sample, size_t sample_size) { - const size_t kBackSearchSize = 1024; - if (sample_size < 64) { + // Maximum size that ValidateJpeg will consider valid. + const size_t kMaxJpegSize = 0x7fffffffull; + if (sample_size < 64 || sample_size > kMaxJpegSize || !sample) { // ERROR: Invalid jpeg size: sample_size return LIBYUV_FALSE; } - if (sample[0] != 0xff || sample[1] != 0xd8) { // Start Of Image + if (sample[0] != 0xff || sample[1] != 0xd8) { // SOI marker // ERROR: Invalid jpeg initial start code return LIBYUV_FALSE; } - // Step over SOI marker. - sample += 2; - sample_size -= 2; - - // Look for the End Of Image (EOI) marker in the end kilobyte of the buffer. - if (sample_size > kBackSearchSize) { - if (ScanEOI(sample + sample_size - kBackSearchSize, kBackSearchSize)) { - return LIBYUV_TRUE; // Success: Valid jpeg. - } - // Reduce search size for forward search. - sample_size = sample_size - kBackSearchSize + 1; - } - return ScanEOI(sample, sample_size); - + // Step over SOI marker and scan for EOI. + return ScanEOI(sample + 2, sample_size - 2); } #ifdef __cplusplus diff --git a/unit_test/convert_test.cc b/unit_test/convert_test.cc index c546e955c..8faacbe52 100644 --- a/unit_test/convert_test.cc +++ b/unit_test/convert_test.cc @@ -1117,12 +1117,16 @@ TEST_F(libyuvTest, ValidateJpeg) { const int kImageSize = benchmark_width_ * benchmark_height_ >= kMinJpeg ? benchmark_width_ * benchmark_height_ : kMinJpeg; const int kSize = kImageSize + kOff; - align_buffer_64(orig_pixels, kSize); + align_buffer_page_end(orig_pixels, kSize); // No SOI or EOI. Expect fail. memset(orig_pixels, 0, kSize); EXPECT_FALSE(ValidateJpeg(orig_pixels, kSize)); + // Test special value that matches marker start. + memset(orig_pixels, 0xff, kSize); + EXPECT_FALSE(ValidateJpeg(orig_pixels, kSize)); + // EOI, SOI. Expect pass. orig_pixels[0] = 0xff; orig_pixels[1] = 0xd8; // SOI. @@ -1142,7 +1146,7 @@ TEST_F(libyuvTest, ValidateJpegLarge) { const int kSize = kImageSize + kOff; const int kMultiple = 10; const int kBufSize = kImageSize * kMultiple + kOff; - align_buffer_64(orig_pixels, kBufSize); + align_buffer_page_end(orig_pixels, kBufSize); // No SOI or EOI. Expect fail. memset(orig_pixels, 0, kBufSize); @@ -1165,7 +1169,16 @@ TEST_F(libyuvTest, InvalidateJpeg) { const int kImageSize = benchmark_width_ * benchmark_height_ >= kMinJpeg ? benchmark_width_ * benchmark_height_ : kMinJpeg; const int kSize = kImageSize + kOff; - align_buffer_64(orig_pixels, kSize); + align_buffer_page_end(orig_pixels, kSize); + + // NULL pointer. Expect fail. + EXPECT_FALSE(ValidateJpeg(NULL, kSize)); + + // Negative size. Expect fail. + EXPECT_FALSE(ValidateJpeg(orig_pixels, -1)); + + // Too large size. Expect fail. + EXPECT_FALSE(ValidateJpeg(orig_pixels, 0xfb000000ull)); // No SOI or EOI. Expect fail. memset(orig_pixels, 0, kSize); @@ -1177,6 +1190,7 @@ TEST_F(libyuvTest, InvalidateJpeg) { for (int times = 0; times < benchmark_iterations_; ++times) { EXPECT_FALSE(ValidateJpeg(orig_pixels, kSize)); } + // EOI but no SOI. Expect fail. orig_pixels[0] = 0; orig_pixels[1] = 0; @@ -1187,20 +1201,37 @@ TEST_F(libyuvTest, InvalidateJpeg) { free_aligned_buffer_page_end(orig_pixels); } +TEST_F(libyuvTest, FuzzJpeg) { + srandom(time(NULL)); + // SOI but no EOI. Expect fail. + for (int times = 0; times < benchmark_iterations_; ++times) { + const int kSize = random() % 5000 + 2; + align_buffer_page_end(orig_pixels, kSize); + MemRandomize(orig_pixels, kSize); + + // Add SOI so frame will be scanned. + orig_pixels[0] = 0xff; + orig_pixels[1] = 0xd8; // SOI. + orig_pixels[kSize - 1] = 0xff; + ValidateJpeg(orig_pixels, kSize); // Failure normally expected. + free_aligned_buffer_page_end(orig_pixels); + } +} + TEST_F(libyuvTest, MJPGToI420) { const int kOff = 10; const int kMinJpeg = 64; const int kImageSize = benchmark_width_ * benchmark_height_ >= kMinJpeg ? benchmark_width_ * benchmark_height_ : kMinJpeg; const int kSize = kImageSize + kOff; - align_buffer_64(orig_pixels, kSize); - align_buffer_64(dst_y_opt, benchmark_width_ * benchmark_height_); - align_buffer_64(dst_u_opt, - SUBSAMPLE(benchmark_width_, 2) * - SUBSAMPLE(benchmark_height_, 2)); - align_buffer_64(dst_v_opt, - SUBSAMPLE(benchmark_width_, 2) * - SUBSAMPLE(benchmark_height_, 2)); + align_buffer_page_end(orig_pixels, kSize); + align_buffer_page_end(dst_y_opt, benchmark_width_ * benchmark_height_); + align_buffer_page_end(dst_u_opt, + SUBSAMPLE(benchmark_width_, 2) * + SUBSAMPLE(benchmark_height_, 2)); + align_buffer_page_end(dst_v_opt, + SUBSAMPLE(benchmark_width_, 2) * + SUBSAMPLE(benchmark_height_, 2)); // EOI, SOI to make MJPG appear valid. memset(orig_pixels, 0, kSize); @@ -1232,8 +1263,8 @@ TEST_F(libyuvTest, MJPGToARGB) { const int kImageSize = benchmark_width_ * benchmark_height_ >= kMinJpeg ? benchmark_width_ * benchmark_height_ : kMinJpeg; const int kSize = kImageSize + kOff; - align_buffer_64(orig_pixels, kSize); - align_buffer_64(dst_argb_opt, benchmark_width_ * benchmark_height_ * 4); + align_buffer_page_end(orig_pixels, kSize); + align_buffer_page_end(dst_argb_opt, benchmark_width_ * benchmark_height_ * 4); // EOI, SOI to make MJPG appear valid. memset(orig_pixels, 0, kSize);