diff options
author | Todd Kennedy <toddke@google.com> | 2018-07-12 13:15:54 -0700 |
---|---|---|
committer | android-build-team Robot <android-build-team-robot@google.com> | 2018-07-17 23:28:27 +0000 |
commit | 19c3315ce0ec83c4e7db1ba2eb5fd7ef2a50ccc5 (patch) | |
tree | df7f4e0a8723a8c7cd6720549452f4975a9876fb | |
parent | e1125aee3ee1a3a92d35fc9bf44de81b73df4d3e (diff) | |
download | base-19c3315ce0ec83c4e7db1ba2eb5fd7ef2a50ccc5.tar.gz |
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)
-rw-r--r-- | libs/androidfw/ChunkIterator.cpp | 21 | ||||
-rw-r--r-- | libs/androidfw/LoadedArsc.cpp | 12 | ||||
-rw-r--r-- | libs/androidfw/include/androidfw/Chunk.h | 9 |
3 files changed, 38 insertions, 4 deletions
diff --git a/libs/androidfw/ChunkIterator.cpp b/libs/androidfw/ChunkIterator.cpp index d8adbe5ac85d..8fc321968055 100644 --- 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 04d506a2d71c..c2740c9fbaa4 100644 --- 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 89b588e2b2e9..99a52dc9244e 100644 --- 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 |