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

Commit ae1c624b authored by Edwin Wong's avatar Edwin Wong
Browse files

[RESTRICT AUTOMERGE] Fix CryptoPlugin use after free vulnerability. 

The shared memory buffer used by srcPtr can be freed by another
thread because it is not protected by a mutex. Subsequently,
a use after free AIGABRT can occur in a race condition.

SafetyNet logging is not added to avoid log spamming. The
mutex lock is called to setup for decryption, which is
called frequently.

Test is run on rvc-dev branch, using target_hwasan-userdebug build.

Test: sts
  sts-tradefed run sts-engbuild-no-spl-lock -m StsHostTestCases --test android.security.sts.Bug_176495665#testPocBug_176495665

Test: push to device with target_hwasan-userdebug build
  adb shell /data/local/tmp/Bug-176495665_sts64

Bug: 176495665
Bug: 176444161
Change-Id: I3ec33cd444183f40ee76bec4c88dec0dac859cd3
parent ef66293e

drm/1.0/default/Android.bp

+2 −1
Original line number Diff line number Diff line
@@ -9,6 +9,7 @@ cc_library_static { 
        "-Werror",

        "-Wextra",

        "-Wall",

        "-Wthread-safety",

    ],

    shared_libs: [

        "liblog",
@@ -19,5 +20,5 @@ cc_library_static { 
    export_header_lib_headers: [

        "libutils_headers",

    ],

    export_include_dirs : ["include"]

    export_include_dirs: ["include"],

}

drm/1.0/default/CryptoPlugin.cpp

+6 −1
Original line number Diff line number Diff line
@@ -54,6 +54,8 @@ namespace implementation { 
        sp<IMemory> hidlMemory = mapMemory(base);

        ALOGE_IF(hidlMemory == nullptr, "mapMemory returns nullptr");



        std::lock_guard<std::mutex> shared_buffer_lock(mSharedBufferLock);



        // allow mapMemory to return nullptr

        mSharedBufferMap[bufferId] = hidlMemory;

        return Void();
@@ -66,7 +68,7 @@ namespace implementation { 
            const SharedBuffer& source, uint64_t offset,

            const DestinationBuffer& destination,

            decrypt_cb _hidl_cb) {



        std::unique_lock<std::mutex> lock(mSharedBufferLock);

        if (mSharedBufferMap.find(source.bufferId) == mSharedBufferMap.end()) {

            _hidl_cb(Status::ERROR_DRM_CANNOT_HANDLE, 0, "source decrypt buffer base not set");

            return Void();
@@ -174,6 +176,9 @@ namespace implementation { 
            _hidl_cb(Status::BAD_VALUE, 0, "invalid destination type");

            return Void();

        }



        // release mSharedBufferLock

        lock.unlock();

        ssize_t result = mLegacyPlugin->decrypt(secure, keyId.data(), iv.data(),

                legacyMode, legacyPattern, srcPtr, legacySubSamples.get(),

                subSamples.size(), destPtr, &detailMessage);

drm/1.0/default/CryptoPlugin.h

+13 −8
Original line number Diff line number Diff line
@@ -17,11 +17,14 @@ 
#ifndef ANDROID_HARDWARE_DRM_V1_0__CRYPTOPLUGIN_H

#define ANDROID_HARDWARE_DRM_V1_0__CRYPTOPLUGIN_H



#include <android/hidl/memory/1.0/IMemory.h>

#include <android-base/thread_annotations.h>

#include <android/hardware/drm/1.0/ICryptoPlugin.h>

#include <android/hidl/memory/1.0/IMemory.h>

#include <hidl/Status.h>

#include <media/hardware/CryptoAPI.h>



#include <mutex>



namespace android {

namespace hardware {

namespace drm {
@@ -60,19 +63,21 @@ struct CryptoPlugin : public ICryptoPlugin { 
    Return<void> setSharedBufferBase(const ::android::hardware::hidl_memory& base,

        uint32_t bufferId) override;



    Return<void> decrypt(bool secure, const hidl_array<uint8_t, 16>& keyId,

            const hidl_array<uint8_t, 16>& iv, Mode mode, const Pattern& pattern,

            const hidl_vec<SubSample>& subSamples, const SharedBuffer& source,

            uint64_t offset, const DestinationBuffer& destination,

            decrypt_cb _hidl_cb) override;

    Return<void> decrypt(

        bool secure, const hidl_array<uint8_t, 16>& keyId, const hidl_array<uint8_t, 16>& iv,

        Mode mode, const Pattern& pattern, const hidl_vec<SubSample>& subSamples,

        const SharedBuffer& source, uint64_t offset, const DestinationBuffer& destination,

        decrypt_cb _hidl_cb) override NO_THREAD_SAFETY_ANALYSIS;  // use unique_lock



   private:

    android::CryptoPlugin *mLegacyPlugin;

    std::map<uint32_t, sp<IMemory> > mSharedBufferMap;

    std::map<uint32_t, sp<IMemory>> mSharedBufferMap GUARDED_BY(mSharedBufferLock);



    CryptoPlugin() = delete;

    CryptoPlugin(const CryptoPlugin &) = delete;

    void operator=(const CryptoPlugin &) = delete;



    std::mutex mSharedBufferLock;

};



}  // namespace implementation