From 14ebc1e5ae6968424eb242f3b0330f82e475a1e4 Mon Sep 17 00:00:00 2001
From: Tianjie Xu <xunchang@google.com>
Date: Wed, 5 Jul 2017 12:04:07 -0700
Subject: Fix a rare failure for imgdiff when random data equals gzip header

In a rare case, a random chunk will pass both the gzip header check
and the inflation process; but fail the uncompressed length check in the
footer. This leads to a imgdiff failure. So, we should treat this chunk
as 'normal' instead of 'inflated' while generating the patch.

Bug: 63334984
Test: imgdiff generates patch successfully on previous failing images.
Change-Id: Ice84f22d3653bce9756bda91e70528c0d2f264a0
---
 applypatch/imgdiff.cpp           | 25 ++++++++++++++-----------
 tests/component/imgdiff_test.cpp | 33 +++++++++++++++++++++++++++++++++
 2 files changed, 47 insertions(+), 11 deletions(-)

diff --git a/applypatch/imgdiff.cpp b/applypatch/imgdiff.cpp
index 41d73ab98..fc240644f 100644
--- a/applypatch/imgdiff.cpp
+++ b/applypatch/imgdiff.cpp
@@ -693,6 +693,20 @@ static bool ReadImage(const char* filename, std::vector<ImageChunk>* chunks,
         continue;
       }
 
+      // The footer contains the size of the uncompressed data.  Double-check to make sure that it
+      // matches the size of the data we got when we actually did the decompression.
+      size_t footer_index = pos + raw_data_len + GZIP_FOOTER_LEN - 4;
+      if (sz - footer_index < 4) {
+        printf("Warning: invalid footer position; treating as a nomal chunk\n");
+        continue;
+      }
+      size_t footer_size = get_unaligned<uint32_t>(img->data() + footer_index);
+      if (footer_size != uncompressed_len) {
+        printf("Warning: footer size %zu != decompressed size %zu; treating as a nomal chunk\n",
+               footer_size, uncompressed_len);
+        continue;
+      }
+
       ImageChunk body(CHUNK_DEFLATE, pos, img, raw_data_len);
       uncompressed_data.resize(uncompressed_len);
       body.SetUncompressedData(std::move(uncompressed_data));
@@ -704,17 +718,6 @@ static bool ReadImage(const char* filename, std::vector<ImageChunk>* chunks,
       chunks->emplace_back(CHUNK_NORMAL, pos, img, GZIP_FOOTER_LEN);
 
       pos += GZIP_FOOTER_LEN;
-
-      // The footer (that we just skipped over) contains the size of
-      // the uncompressed data.  Double-check to make sure that it
-      // matches the size of the data we got when we actually did
-      // the decompression.
-      size_t footer_size = get_unaligned<uint32_t>(img->data() + pos - 4);
-      if (footer_size != body.DataLengthForPatch()) {
-        printf("Error: footer size %zu != decompressed size %zu\n", footer_size,
-               body.GetRawDataLength());
-        return false;
-      }
     } else {
       // Use a normal chunk to take all the contents until the next gzip chunk (or EOF); we expect
       // the number of chunks to be small (5 for typical boot and recovery images).
diff --git a/tests/component/imgdiff_test.cpp b/tests/component/imgdiff_test.cpp
index 6f5960bbd..bf25aebb0 100644
--- a/tests/component/imgdiff_test.cpp
+++ b/tests/component/imgdiff_test.cpp
@@ -328,6 +328,39 @@ TEST(ImgdiffTest, image_mode_simple) {
   verify_patched_image(src, patch, tgt);
 }
 
+TEST(ImgdiffTest, image_mode_bad_gzip) {
+  // Modify the uncompressed length in the gzip footer.
+  const std::vector<char> src_data = { 'a',    'b',    'c',    'd',    'e',    'f',    'g',
+                                       'h',    '\x1f', '\x8b', '\x08', '\x00', '\xc4', '\x1e',
+                                       '\x53', '\x58', '\x00', '\x03', '\xab', '\xa8', '\xac',
+                                       '\x02', '\x00', '\x67', '\xba', '\x8e', '\xeb', '\x03',
+                                       '\xff', '\xff', '\xff' };
+  const std::string src(src_data.cbegin(), src_data.cend());
+  TemporaryFile src_file;
+  ASSERT_TRUE(android::base::WriteStringToFile(src, src_file.path));
+
+  // Modify the uncompressed length in the gzip footer.
+  const std::vector<char> tgt_data = {
+    'a',    'b',    'c',    'd',    'e',    'f',    'g',    'x',    'y',    'z',    '\x1f', '\x8b',
+    '\x08', '\x00', '\x62', '\x1f', '\x53', '\x58', '\x00', '\x03', '\xab', '\xa8', '\xa8', '\xac',
+    '\xac', '\xaa', '\x02', '\x00', '\x96', '\x30', '\x06', '\xb7', '\x06', '\xff', '\xff', '\xff'
+  };
+  const std::string tgt(tgt_data.cbegin(), tgt_data.cend());
+  TemporaryFile tgt_file;
+  ASSERT_TRUE(android::base::WriteStringToFile(tgt, tgt_file.path));
+
+  TemporaryFile patch_file;
+  std::vector<const char*> args = {
+    "imgdiff", src_file.path, tgt_file.path, patch_file.path,
+  };
+  ASSERT_EQ(0, imgdiff(args.size(), args.data()));
+
+  // Verify.
+  std::string patch;
+  ASSERT_TRUE(android::base::ReadFileToString(patch_file.path, &patch));
+  verify_patched_image(src, patch, tgt);
+}
+
 TEST(ImgdiffTest, image_mode_different_num_chunks) {
   // src: "abcdefgh" + gzipped "xyz" (echo -n "xyz" | gzip -f | hd) + gzipped "test".
   const std::vector<char> src_data = {
-- 
cgit v1.2.3