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

Commit 7a398e72 authored by Edwin Wong's avatar Edwin Wong
Browse files

[RESTRICT AUTOMERGE] Fix clearkey 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.

The crash was reproduced on the device before the fix.
Verified the test passes after the fix.

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: Ie6e73804d8b764e53b1bd7a16cfbf04b4f3669b9
parent 746c14a7
Loading
Loading
Loading
Loading
+2 −1
Original line number Diff line number Diff line
@@ -36,7 +36,8 @@ cc_binary {

    relative_install_path: "hw",

    cflags: ["-Wall", "-Werror"],
    cflags: ["-Wall", "-Werror", "-Wthread-safety"],

    init_rc: ["android.hardware.drm@1.1-service.clearkey.rc"],

    shared_libs: [
+6 −0
Original line number Diff line number Diff line
@@ -37,6 +37,8 @@ Return<void> CryptoPlugin::setSharedBufferBase(
    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();
@@ -64,6 +66,7 @@ Return<void> CryptoPlugin::decrypt(
        return Void();
    }

    std::unique_lock<std::mutex> shared_buffer_lock(mSharedBufferLock);
    if (mSharedBufferMap.find(source.bufferId) == mSharedBufferMap.end()) {
      _hidl_cb(Status::ERROR_DRM_CANNOT_HANDLE, 0,
               "source decrypt buffer base not set");
@@ -120,6 +123,9 @@ Return<void> CryptoPlugin::decrypt(
    }
    destPtr = static_cast<void*>(base + destination.nonsecureMemory.offset);

    // release mSharedBufferLock
    shared_buffer_lock.unlock();

    // Calculate the output buffer size and determine if any subsamples are
    // encrypted.
    size_t destSize = 0;
+5 −2
Original line number Diff line number Diff line
@@ -20,6 +20,8 @@
#include <android/hardware/drm/1.0/ICryptoPlugin.h>
#include <android/hidl/memory/1.0/IMemory.h>

#include <mutex>

#include "ClearKeyTypes.h"
#include "Session.h"
#include "Utils.h"
@@ -78,7 +80,7 @@ struct CryptoPlugin : public ICryptoPlugin {
            const SharedBuffer& source,
            uint64_t offset,
            const DestinationBuffer& destination,
            decrypt_cb _hidl_cb);
            decrypt_cb _hidl_cb) NO_THREAD_SAFETY_ANALYSIS; // use unique_lock

    Return<void> setSharedBufferBase(const hidl_memory& base,
            uint32_t bufferId);
@@ -90,7 +92,8 @@ struct CryptoPlugin : public ICryptoPlugin {
private:
    CLEARKEY_DISALLOW_COPY_AND_ASSIGN(CryptoPlugin);

    std::map<uint32_t, sp<IMemory> > mSharedBufferMap;
    std::mutex mSharedBufferLock;
    std::map<uint32_t, sp<IMemory>> mSharedBufferMap GUARDED_BY(mSharedBufferLock);
    sp<Session> mSession;
    Status mInitStatus;
};