commit 67ea2b745b1b9491df42e5bd73d9320cb0a2f0fa
author Kelvin Zhang <zhangkelvin@google.com> Wed May 17 19:56:00 2023 +0000
committer Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com> Wed May 17 19:56:00 2023 +0000
tree 8a93bda23e76dea8bdeed55a53f37ed389b5af7d
parent d59428bae7602b3865dc22bb36ada746aab719c4
parent b78bfc3d3dde30927090587576b8a6224d03851c

Fix boot image parsing when input image is too small am: 010c6d1468 am: b78bfc3d3d

Original change: https://android-review.googlesource.com/c/platform/system/update_engine/+/2593466

Change-Id: If591071ecad4738955aa58a5d61863bb79fcc7ea
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>

payload_generator/boot_img_filesystem.cc

diff --git a/payload_generator/boot_img_filesystem.cc b/payload_generator/boot_img_filesystem.cc
index cad267e..3e324f1 100644
--- a/payload_generator/boot_img_filesystem.cc
+++ b/payload_generator/boot_img_filesystem.cc
@@ -16,10 +16,15 @@
 
 #include "update_engine/payload_generator/boot_img_filesystem.h"
 
+#include <array>
+
+#include <android-base/unique_fd.h>
+#include <android-base/file.h>
 #include <base/logging.h>
 #include <bootimg.h>
 #include <brillo/secure_blob.h>
 #include <puffin/utils.h>
+#include <sys/stat.h>
 
 #include "update_engine/common/utils.h"
 #include "update_engine/payload_generator/delta_diff_generator.h"
@@ -33,11 +38,26 @@
 
 unique_ptr<BootImgFilesystem> BootImgFilesystem::CreateFromFile(
     const string& filename) {
-  if (filename.empty())
+  if (filename.empty()) {
     return nullptr;
-
-  if (brillo::Blob header_magic;
-      !utils::ReadFileChunk(filename, 0, BOOT_MAGIC_SIZE, &header_magic) ||
+  }
+  android::base::unique_fd fd(open(filename.c_str(), O_RDONLY));
+  if (!fd.ok()) {
+    PLOG(ERROR) << "Failed to open " << filename;
+    return nullptr;
+  }
+  struct stat st {};
+  if (fstat(fd.get(), &st) < 0) {
+    return nullptr;
+  }
+  if (static_cast<size_t>(st.st_size) < sizeof(boot_img_hdr_v0)) {
+    LOG(INFO) << "Image " << filename
+              << " is too small to be a boot image. file size: " << st.st_size;
+    return nullptr;
+  }
+  std::array<char, BOOT_MAGIC_SIZE> header_magic{};
+  if (!android::base::ReadFullyAtOffset(
+          fd, header_magic.data(), BOOT_MAGIC_SIZE, 0) ||
       memcmp(header_magic.data(), BOOT_MAGIC, BOOT_MAGIC_SIZE) != 0) {
     return nullptr;
   }
@@ -48,11 +68,11 @@
   // See details in system/tools/mkbootimg/include/bootimg/bootimg.h
   constexpr size_t header_version_offset =
       BOOT_MAGIC_SIZE + 8 * sizeof(uint32_t);
-  brillo::Blob header_version_blob;
-  if (!utils::ReadFileChunk(filename,
-                            header_version_offset,
-                            sizeof(uint32_t),
-                            &header_version_blob)) {
+  std::array<char, sizeof(uint32_t)> header_version_blob{};
+  if (!android::base::ReadFullyAtOffset(fd,
+                                        header_version_blob.data(),
+                                        sizeof(uint32_t),
+                                        header_version_offset)) {
     return nullptr;
   }
   uint32_t header_version =
@@ -66,8 +86,8 @@
   // Read the bytes of boot image header based on the header version.
   size_t header_size =
       header_version == 3 ? sizeof(boot_img_hdr_v3) : sizeof(boot_img_hdr_v0);
-  brillo::Blob header_blob;
-  if (!utils::ReadFileChunk(filename, 0, header_size, &header_blob)) {
+  brillo::Blob header_blob(header_size);
+  if (!ReadFullyAtOffset(fd, header_blob.data(), header_size, 0)) {
     return nullptr;
   }
 
@@ -120,7 +140,8 @@
   file.extents = {ExtentForBytes(kBlockSize, offset, size)};
 
   brillo::Blob data;
-  if (utils::ReadFileChunk(filename_, offset, size, &data)) {
+  if (utils::ReadFileChunk(filename_, offset, size, &data) &&
+      data.size() == size) {
     constexpr size_t kGZipHeaderSize = 10;
     // Check GZip header magic.
     if (data.size() > kGZipHeaderSize && data[0] == 0x1F && data[1] == 0x8B) {