Loosen resource file verification

Bug: 77808145
Test: Tried to install corrupt APK prior to the change, install failed
Test: Tried to install corrupt APK after the change, install succeeded
Test: atest CtsAppSecurityHostTestCases:CorruptApkTests
Change-Id: I19a69e52a17c1080beaf2cc575c32f564b1033a3
(cherry picked from commit 28e663cbed28fb6c8c8dec0849e0277daf67651b)

diff --git a/libs/androidfw/ChunkIterator.cpp b/libs/androidfw/ChunkIterator.cpp
index d8adbe5..8fc3219 100644 (file)
--- a/libs/androidfw/ChunkIterator.cpp
+++ b/libs/androidfw/ChunkIterator.cpp
@@ -32,11 +32,30 @@ Chunk ChunkIterator::Next() {
 
   if (len_ != 0) {
     // Prepare the next chunk.
-    VerifyNextChunk();
+    if (VerifyNextChunkNonFatal()) {
+      VerifyNextChunk();
+    }
   }
   return Chunk(this_chunk);
 }
 
+// TODO(b/111401637) remove this and have full resource file verification
+// Returns false if there was an error.
+bool ChunkIterator::VerifyNextChunkNonFatal() {
+  if (len_ < sizeof(ResChunk_header)) {
+    last_error_ = "not enough space for header";
+    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";
+    last_error_was_fatal_ = false;
+    return false;
+  }
+  return true;
+}
+
 // Returns false if there was an error.
 bool ChunkIterator::VerifyNextChunk() {
   const uintptr_t header_start = reinterpret_cast<uintptr_t>(next_chunk_);

diff --git a/libs/androidfw/LoadedArsc.cpp b/libs/androidfw/LoadedArsc.cpp
index 04d506a..c2740c9 100644 (file)
--- a/libs/androidfw/LoadedArsc.cpp
+++ b/libs/androidfw/LoadedArsc.cpp
@@ -560,7 +560,9 @@ std::unique_ptr<const LoadedPackage> LoadedPackage::Load(const Chunk& chunk,
 
   if (iter.HadError()) {
     LOG(ERROR) << iter.GetLastError();
-    return {};
+    if (iter.HadFatalError()) {
+      return {};
+    }
   }
 
   // Flatten and construct the TypeSpecs.
@@ -641,7 +643,9 @@ bool LoadedArsc::LoadTable(const Chunk& chunk, const LoadedIdmap* loaded_idmap,
 
   if (iter.HadError()) {
     LOG(ERROR) << iter.GetLastError();
-    return false;
+    if (iter.HadFatalError()) {
+      return false;
+    }
   }
   return true;
 }
@@ -673,7 +677,9 @@ std::unique_ptr<const LoadedArsc> LoadedArsc::Load(const StringPiece& data,
 
   if (iter.HadError()) {
     LOG(ERROR) << iter.GetLastError();
-    return {};
+    if (iter.HadFatalError()) {
+      return {};
+    }
   }
 
   // Need to force a move for mingw32.

diff --git a/libs/androidfw/include/androidfw/Chunk.h b/libs/androidfw/include/androidfw/Chunk.h
index 89b588e..99a52dc 100644 (file)
--- a/libs/androidfw/include/androidfw/Chunk.h
+++ b/libs/androidfw/include/androidfw/Chunk.h
@@ -94,18 +94,27 @@ class ChunkIterator {
 
   Chunk Next();
   inline bool HasNext() const { return !HadError() && len_ != 0; };
+  // Returns whether there was an error and processing should stop
   inline bool HadError() const { return last_error_ != nullptr; }
   inline std::string GetLastError() const { return last_error_; }
+  // Returns whether there was an error and processing should stop. For legacy purposes,
+  // some errors are considered "non fatal". Fatal errors stop processing new chunks and
+  // throw away any chunks already processed. Non fatal errors also stop processing new
+  // chunks, but, will retain and use any valid chunks already processed.
+  inline bool HadFatalError() const { return HadError() && last_error_was_fatal_; }
 
  private:
   DISALLOW_COPY_AND_ASSIGN(ChunkIterator);
 
   // Returns false if there was an error.
   bool VerifyNextChunk();
+  // Returns false if there was an error. For legacy purposes.
+  bool VerifyNextChunkNonFatal();
 
   const ResChunk_header* next_chunk_;
   size_t len_;
   const char* last_error_;
+  bool last_error_was_fatal_ = true;
 };
 
 }  // namespace android