Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit d37ce8f0 authored by Tao Bao's avatar Tao Bao
Browse files

imgdiff: Fix an edge case that leads to infinite loop. 

When the input image ends with the magic value sequence of 0x1f, 0x8b,
0x0b (optionally with 0x00), the image parsing code will be stuck in an
infinite loop.

Test: recovery_component_test passes.
Change-Id: Ie3629dfdc41360387b19cc3e0359c95ae4fb998e
parent 344c8eb4

applypatch/Android.mk

+1 −0
Original line number Diff line number Diff line
@@ -124,6 +124,7 @@ libimgdiff_cflags := \ 


libimgdiff_static_libraries := \

    libbsdiff \

    libbase \

    libz



# libimgdiff (static library)

applypatch/imgdiff.cpp

+10 −15
Original line number Diff line number Diff line
@@ -124,6 +124,7 @@ 
#include "applypatch/imgdiff.h"



#include <errno.h>

#include <fcntl.h>

#include <stdio.h>

#include <stdlib.h>

#include <string.h>
@@ -131,6 +132,9 @@ 
#include <sys/types.h>

#include <unistd.h>



#include <android-base/file.h>

#include <android-base/unique_fd.h>



#include <bsdiff.h>

#include <zlib.h>
@@ -382,19 +386,12 @@ unsigned char* ReadImage(const char* filename, int* num_chunks, ImageChunk** chu 
  }



  size_t sz = static_cast<size_t>(st.st_size);

  unsigned char* img = static_cast<unsigned char*>(malloc(sz + 4));

  FILE* f = fopen(filename, "rb");

  if (fread(img, 1, sz, f) != sz) {

  unsigned char* img = static_cast<unsigned char*>(malloc(sz));

  android::base::unique_fd fd(open(filename, O_RDONLY));

  if (!android::base::ReadFully(fd, img, sz)) {

    printf("failed to read \"%s\" %s\n", filename, strerror(errno));

    fclose(f);

    return NULL;

    return nullptr;

  }

  fclose(f);



  // append 4 zero bytes to the data so we can always search for the

  // four-byte string 1f8b0800 starting at any point in the actual

  // file data, without special-casing the end of the data.

  memset(img+sz, 0, 4);



  size_t pos = 0;
@@ -518,10 +515,8 @@ unsigned char* ReadImage(const char* filename, int* num_chunks, ImageChunk** chu 
      curr->data = p;



      for (curr->len = 0; curr->len < (sz - pos); ++curr->len) {

        if (p[curr->len] == 0x1f &&

            p[curr->len+1] == 0x8b &&

            p[curr->len+2] == 0x08 &&

            p[curr->len+3] == 0x00) {

        if (sz - pos >= 4 && p[curr->len] == 0x1f && p[curr->len + 1] == 0x8b &&

            p[curr->len + 2] == 0x08 && p[curr->len + 3] == 0x00) {

          break;

        }

      }

tests/component/imgdiff_test.cpp

+80 −0
Original line number Diff line number Diff line
@@ -404,6 +404,86 @@ TEST(ImgdiffTest, image_mode_spurious_magic) { 
  ASSERT_EQ(tgt, patched);

}



TEST(ImgdiffTest, image_mode_short_input1) {

  // src: "abcdefgh" + '0x1f8b0b'.

  const std::vector<char> src_data = { 'a', 'b', 'c',    'd',    'e',   'f',

                                       'g', 'h', '\x1f', '\x8b', '\x08' };

  const std::string src(src_data.cbegin(), src_data.cend());

  TemporaryFile src_file;

  ASSERT_TRUE(android::base::WriteStringToFile(src, src_file.path));



  // tgt: "abcdefgxyz".

  const std::vector<char> tgt_data = { 'a', 'b', 'c', 'd', 'e', 'f', 'g', 'x', 'y', 'z' };

  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));



  // Expect one CHUNK_RAW (header) entry.

  size_t num_normal;

  size_t num_raw;

  size_t num_deflate;

  verify_patch_header(patch, &num_normal, &num_raw, &num_deflate);

  ASSERT_EQ(0U, num_normal);

  ASSERT_EQ(0U, num_deflate);

  ASSERT_EQ(1U, num_raw);



  std::string patched;

  ASSERT_EQ(0, ApplyImagePatch(reinterpret_cast<const unsigned char*>(src.data()), src.size(),

                               reinterpret_cast<const unsigned char*>(patch.data()), patch.size(),

                               MemorySink, &patched));

  ASSERT_EQ(tgt, patched);

}



TEST(ImgdiffTest, image_mode_short_input2) {

  // src: "abcdefgh" + '0x1f8b0b00'.

  const std::vector<char> src_data = { 'a', 'b', 'c',    'd',    'e',    'f',

                                       'g', 'h', '\x1f', '\x8b', '\x08', '\x00' };

  const std::string src(src_data.cbegin(), src_data.cend());

  TemporaryFile src_file;

  ASSERT_TRUE(android::base::WriteStringToFile(src, src_file.path));



  // tgt: "abcdefgxyz".

  const std::vector<char> tgt_data = { 'a', 'b', 'c', 'd', 'e', 'f', 'g', 'x', 'y', 'z' };

  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));



  // Expect one CHUNK_RAW (header) entry.

  size_t num_normal;

  size_t num_raw;

  size_t num_deflate;

  verify_patch_header(patch, &num_normal, &num_raw, &num_deflate);

  ASSERT_EQ(0U, num_normal);

  ASSERT_EQ(0U, num_deflate);

  ASSERT_EQ(1U, num_raw);



  std::string patched;

  ASSERT_EQ(0, ApplyImagePatch(reinterpret_cast<const unsigned char*>(src.data()), src.size(),

                               reinterpret_cast<const unsigned char*>(patch.data()), patch.size(),

                               MemorySink, &patched));

  ASSERT_EQ(tgt, patched);

}



TEST(ImgdiffTest, image_mode_single_entry_long) {

  // src: "abcdefgh" + '0x1f8b0b00' + some bytes.

  const std::vector<char> src_data = { 'a',    'b',    'c',    'd',    'e',    'f',    'g',