Fail installation in case zip file is malformed.
Currently we only fail install if we can't open zip file. We need to
also fail if there were issues traversing the content.
Bug: 288609769
Fixes: 288609769
Test: atest libandroidfw_tests
Change-Id: I37c62b485ec145a12ed8e5f637dc7c7f6a68110e
diff --git a/core/jni/com_android_internal_content_NativeLibraryHelper.cpp b/core/jni/com_android_internal_content_NativeLibraryHelper.cpp
index 9017d58..da308b2 100644
--- a/core/jni/com_android_internal_content_NativeLibraryHelper.cpp
+++ b/core/jni/com_android_internal_content_NativeLibraryHelper.cpp
@@ -292,21 +292,31 @@
}
public:
- static NativeLibrariesIterator* create(ZipFileRO* zipFile, bool debuggable) {
- void* cookie = nullptr;
+ static base::expected<std::unique_ptr<NativeLibrariesIterator>, int32_t> create(
+ ZipFileRO* zipFile, bool debuggable) {
// Do not specify a suffix to find both .so files and gdbserver.
- if (!zipFile->startIteration(&cookie, APK_LIB.data(), nullptr /* suffix */)) {
- return nullptr;
+ auto result = zipFile->startIterationOrError(APK_LIB.data(), nullptr /* suffix */);
+ if (!result.ok()) {
+ return base::unexpected(result.error());
}
- return new NativeLibrariesIterator(zipFile, debuggable, cookie);
+ return std::unique_ptr<NativeLibrariesIterator>(
+ new NativeLibrariesIterator(zipFile, debuggable, result.value()));
}
- ZipEntryRO next() {
- ZipEntryRO next = nullptr;
- while ((next = mZipFile->nextEntry(mCookie)) != nullptr) {
+ base::expected<ZipEntryRO, int32_t> next() {
+ ZipEntryRO nextEntry;
+ while (true) {
+ auto next = mZipFile->nextEntryOrError(mCookie);
+ if (!next.ok()) {
+ return base::unexpected(next.error());
+ }
+ nextEntry = next.value();
+ if (nextEntry == nullptr) {
+ break;
+ }
// Make sure this entry has a filename.
- if (mZipFile->getEntryFileName(next, fileName, sizeof(fileName))) {
+ if (mZipFile->getEntryFileName(nextEntry, fileName, sizeof(fileName))) {
continue;
}
@@ -317,7 +327,7 @@
}
}
- return next;
+ return nextEntry;
}
inline const char* currentEntry() const {
@@ -348,19 +358,28 @@
return INSTALL_FAILED_INVALID_APK;
}
- std::unique_ptr<NativeLibrariesIterator> it(
- NativeLibrariesIterator::create(zipFile, debuggable));
- if (it.get() == nullptr) {
+ auto result = NativeLibrariesIterator::create(zipFile, debuggable);
+ if (!result.ok()) {
return INSTALL_FAILED_INVALID_APK;
}
+ std::unique_ptr<NativeLibrariesIterator> it(std::move(result.value()));
const ScopedUtfChars cpuAbi(env, javaCpuAbi);
if (cpuAbi.c_str() == nullptr) {
// This would've thrown, so this return code isn't observable by Java.
return INSTALL_FAILED_INVALID_APK;
}
- ZipEntryRO entry = nullptr;
- while ((entry = it->next()) != nullptr) {
+
+ while (true) {
+ auto next = it->next();
+ if (!next.ok()) {
+ return INSTALL_FAILED_INVALID_APK;
+ }
+ auto entry = next.value();
+ if (entry == nullptr) {
+ break;
+ }
+
const char* fileName = it->currentEntry();
const char* lastSlash = it->lastSlash();
@@ -388,11 +407,11 @@
return INSTALL_FAILED_INVALID_APK;
}
- std::unique_ptr<NativeLibrariesIterator> it(
- NativeLibrariesIterator::create(zipFile, debuggable));
- if (it.get() == nullptr) {
+ auto result = NativeLibrariesIterator::create(zipFile, debuggable);
+ if (!result.ok()) {
return INSTALL_FAILED_INVALID_APK;
}
+ std::unique_ptr<NativeLibrariesIterator> it(std::move(result.value()));
const int numAbis = env->GetArrayLength(supportedAbisArray);
@@ -402,9 +421,17 @@
supportedAbis.emplace_back(env, (jstring)env->GetObjectArrayElement(supportedAbisArray, i));
}
- ZipEntryRO entry = nullptr;
int status = NO_NATIVE_LIBRARIES;
- while ((entry = it->next()) != nullptr) {
+ while (true) {
+ auto next = it->next();
+ if (!next.ok()) {
+ return INSTALL_FAILED_INVALID_APK;
+ }
+ auto entry = next.value();
+ if (entry == nullptr) {
+ break;
+ }
+
// We're currently in the lib/ directory of the APK, so it does have some native
// code. We should return INSTALL_FAILED_NO_MATCHING_ABIS if none of the
// libraries match.
diff --git a/libs/androidfw/ZipFileRO.cpp b/libs/androidfw/ZipFileRO.cpp
index 52e7a70..d7b5914 100644
--- a/libs/androidfw/ZipFileRO.cpp
+++ b/libs/androidfw/ZipFileRO.cpp
@@ -40,17 +40,24 @@
public:
ZipEntry entry;
std::string_view name;
- void *cookie;
+ void *cookie = nullptr;
- _ZipEntryRO() : cookie(NULL) {}
+ _ZipEntryRO() = default;
~_ZipEntryRO() {
- EndIteration(cookie);
+ EndIteration(cookie);
+ }
+
+ android::ZipEntryRO convertToPtr() {
+ _ZipEntryRO* result = new _ZipEntryRO;
+ result->entry = std::move(this->entry);
+ result->name = std::move(this->name);
+ result->cookie = std::exchange(this->cookie, nullptr);
+ return result;
}
private:
- _ZipEntryRO(const _ZipEntryRO& other);
- _ZipEntryRO& operator=(const _ZipEntryRO& other);
+ DISALLOW_COPY_AND_ASSIGN(_ZipEntryRO);
};
ZipFileRO::~ZipFileRO() {
@@ -94,17 +101,15 @@
ZipEntryRO ZipFileRO::findEntryByName(const char* entryName) const
{
- _ZipEntryRO* data = new _ZipEntryRO;
+ _ZipEntryRO data;
+ data.name = entryName;
- data->name = entryName;
-
- const int32_t error = FindEntry(mHandle, entryName, &(data->entry));
+ const int32_t error = FindEntry(mHandle, entryName, &(data.entry));
if (error) {
- delete data;
- return NULL;
+ return nullptr;
}
- return (ZipEntryRO) data;
+ return data.convertToPtr();
}
/*
@@ -143,35 +148,50 @@
}
bool ZipFileRO::startIteration(void** cookie) {
- return startIteration(cookie, NULL, NULL);
+ return startIteration(cookie, nullptr, nullptr);
}
-bool ZipFileRO::startIteration(void** cookie, const char* prefix, const char* suffix)
-{
- _ZipEntryRO* ze = new _ZipEntryRO;
- int32_t error = StartIteration(mHandle, &(ze->cookie),
+bool ZipFileRO::startIteration(void** cookie, const char* prefix, const char* suffix) {
+ auto result = startIterationOrError(prefix, suffix);
+ if (!result.ok()) {
+ return false;
+ }
+ *cookie = result.value();
+ return true;
+}
+
+base::expected<void*, int32_t>
+ZipFileRO::startIterationOrError(const char* prefix, const char* suffix) {
+ _ZipEntryRO ze;
+ int32_t error = StartIteration(mHandle, &(ze.cookie),
prefix ? prefix : "", suffix ? suffix : "");
if (error) {
ALOGW("Could not start iteration over %s: %s", mFileName != NULL ? mFileName : "<null>",
ErrorCodeString(error));
- delete ze;
- return false;
+ return base::unexpected(error);
}
- *cookie = ze;
- return true;
+ return ze.convertToPtr();
}
-ZipEntryRO ZipFileRO::nextEntry(void* cookie)
-{
+ZipEntryRO ZipFileRO::nextEntry(void* cookie) {
+ auto result = nextEntryOrError(cookie);
+ if (!result.ok()) {
+ return nullptr;
+ }
+ return result.value();
+}
+
+base::expected<ZipEntryRO, int32_t> ZipFileRO::nextEntryOrError(void* cookie) {
_ZipEntryRO* ze = reinterpret_cast<_ZipEntryRO*>(cookie);
int32_t error = Next(ze->cookie, &(ze->entry), &(ze->name));
if (error) {
if (error != -1) {
ALOGW("Error iteration over %s: %s", mFileName != NULL ? mFileName : "<null>",
ErrorCodeString(error));
+ return base::unexpected(error);
}
- return NULL;
+ return nullptr;
}
return &(ze->entry);
diff --git a/libs/androidfw/include/androidfw/ZipFileRO.h b/libs/androidfw/include/androidfw/ZipFileRO.h
index 10f6d06..be1f98f 100644
--- a/libs/androidfw/include/androidfw/ZipFileRO.h
+++ b/libs/androidfw/include/androidfw/ZipFileRO.h
@@ -37,6 +37,8 @@
#include <unistd.h>
#include <time.h>
+#include <android-base/expected.h>
+
#include <util/map_ptr.h>
#include <utils/Compat.h>
@@ -102,6 +104,11 @@
*/
bool startIteration(void** cookie);
bool startIteration(void** cookie, const char* prefix, const char* suffix);
+ /*
+ * Same as above, but returns the error code in case of failure.
+ * #see libziparchive/zip_error.h.
+ */
+ base::expected<void*, int32_t> startIterationOrError(const char* prefix, const char* suffix);
/**
* Return the next entry in iteration order, or NULL if there are no more
@@ -109,6 +116,12 @@
*/
ZipEntryRO nextEntry(void* cookie);
+ /**
+ * Same as above, but returns the error code in case of failure.
+ * #see libziparchive/zip_error.h.
+ */
+ base::expected<ZipEntryRO, int32_t> nextEntryOrError(void* cookie);
+
void endIteration(void* cookie);
void releaseEntry(ZipEntryRO entry) const;