commit db21f09a8e02bcfd3fefea68084667688268f1fa
author Ryan Mitchell <rtmitchell@google.com> Mon Nov 16 23:08:18 2020 +0000
committer Ryan Mitchell <rtmitchell@google.com> Tue Nov 17 23:01:35 2020 +0000
tree c9b22a2676936eaf56ee1160c9349632ccd7ba44
parent 687f5e163f24ff31f822f986fd7a99b8832b6286

Revert^2 "libandroidfw hardening for IncFs"

55ef6167a2c235bd88c7216238b2001b46795b79

Change-Id: I02d4890d181655dfd0a14c188468db512559d27b

libs/androidfw/ChunkIterator.cpp

diff --git a/libs/androidfw/ChunkIterator.cpp b/libs/androidfw/ChunkIterator.cpp
index 8fc3219..25c8aa6 100644
--- a/libs/androidfw/ChunkIterator.cpp
+++ b/libs/androidfw/ChunkIterator.cpp
@@ -15,6 +15,7 @@
  */
 
 #include "androidfw/Chunk.h"
+#include "androidfw/Util.h"
 
 #include "android-base/logging.h"
 
@@ -23,11 +24,11 @@
 Chunk ChunkIterator::Next() {
   CHECK(len_ != 0) << "called Next() after last chunk";
 
-  const ResChunk_header* this_chunk = next_chunk_;
+  const incfs::map_ptr<ResChunk_header> this_chunk = next_chunk_;
+  CHECK((bool) this_chunk) << "Next() called without verifying next chunk";
 
   // We've already checked the values of this_chunk, so safely increment.
-  next_chunk_ = reinterpret_cast<const ResChunk_header*>(
-      reinterpret_cast<const uint8_t*>(this_chunk) + dtohl(this_chunk->size));
+  next_chunk_ = this_chunk.offset(dtohl(this_chunk->size)).convert<ResChunk_header>();
   len_ -= dtohl(this_chunk->size);
 
   if (len_ != 0) {
@@ -36,7 +37,7 @@
       VerifyNextChunk();
     }
   }
-  return Chunk(this_chunk);
+  return Chunk(this_chunk.verified());
 }
 
 // TODO(b/111401637) remove this and have full resource file verification
@@ -47,6 +48,13 @@
     last_error_was_fatal_ = false;
     return false;
   }
+
+  if (!next_chunk_) {
+    last_error_ = "failed to read chunk from data";
+    last_error_was_fatal_ = false;
+    return false;
+  }
+
   const size_t size = dtohl(next_chunk_->size);
   if (size > len_) {
     last_error_ = "chunk size is bigger than given data";
@@ -58,12 +66,10 @@
 
 // Returns false if there was an error.
 bool ChunkIterator::VerifyNextChunk() {
-  const uintptr_t header_start = reinterpret_cast<uintptr_t>(next_chunk_);
-
   // This data must be 4-byte aligned, since we directly
   // access 32-bit words, which must be aligned on
   // certain architectures.
-  if (header_start & 0x03) {
+  if (!util::IsFourByteAligned(next_chunk_)) {
     last_error_ = "header not aligned on 4-byte boundary";
     return false;
   }
@@ -73,6 +79,11 @@
     return false;
   }
 
+  if (!next_chunk_) {
+    last_error_ = "failed to read chunk from data";
+    return false;
+  }
+
   const size_t header_size = dtohs(next_chunk_->headerSize);
   const size_t size = dtohl(next_chunk_->size);
   if (header_size < sizeof(ResChunk_header)) {
@@ -90,7 +101,7 @@
     return false;
   }
 
-  if ((size | header_size) & 0x03) {
+  if ((size | header_size) & 0x03U) {
     last_error_ = "header sizes are not aligned on 4-byte boundary";
     return false;
   }