Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit b92c16e2 authored by Alex Buynytskyy's avatar Alex Buynytskyy
Browse files

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
parent 23843021

core/jni/com_android_internal_content_NativeLibraryHelper.cpp

+47 −20
Original line number Diff line number Diff line
@@ -292,21 +292,31 @@ private: 
    }



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 @@ public: 
            }

        }



        return next;

        return nextEntry;

    }



    inline const char* currentEntry() const {
@@ -348,19 +358,28 @@ iterateOverNativeFiles(JNIEnv *env, jlong apkHandle, jstring javaCpuAbi, 
        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 @@ static int findSupportedAbi(JNIEnv* env, jlong apkHandle, jobjectArray supported 
        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 @@ static int findSupportedAbi(JNIEnv* env, jlong apkHandle, jobjectArray supported 
        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.

libs/androidfw/ZipFileRO.cpp

+44 −24
Original line number Diff line number Diff line
@@ -40,17 +40,24 @@ class _ZipEntryRO { 
public:

    ZipEntry entry;

    std::string_view name;

    void *cookie;

    void *cookie = nullptr;



    _ZipEntryRO() : cookie(NULL) {}

    _ZipEntryRO() = default;



    ~_ZipEntryRO() {

        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 @@ ZipFileRO::~ZipFileRO() { 


ZipEntryRO ZipFileRO::findEntryByName(const char* entryName) const

{

    _ZipEntryRO* data = new _ZipEntryRO;



    data->name = entryName;

    _ZipEntryRO data;

    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::getEntryInfo(ZipEntryRO entry, uint16_t* pMethod, 
}



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);

libs/androidfw/include/androidfw/ZipFileRO.h

+13 −0
Original line number Diff line number Diff line
@@ -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 @@ public: 
     */

    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 @@ public: 
     */

    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;