From 596d8bf4cc9071e2f767bf926b97a66035608ede Mon Sep 17 00:00:00 2001
From: Edwin Wong <edwinwong@google.com>
Date: Mon, 8 Mar 2021 21:37:42 -0800
Subject: [PATCH] Fix WVCryptoPlugin 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: If62b73a9c636048f942a2fc63a13b5bfd1e57b86
---
 libwvdrmengine/mediacrypto/Android.bp                    | 2 ++
 libwvdrmengine/mediacrypto/include_hidl/WVCryptoPlugin.h | 9 +++++++--
 libwvdrmengine/mediacrypto/src_hidl/WVCryptoPlugin.cpp   | 7 ++++++-
 3 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/libwvdrmengine/mediacrypto/Android.bp b/libwvdrmengine/mediacrypto/Android.bp
index 8f9b9e78..9effe8af 100644
--- a/libwvdrmengine/mediacrypto/Android.bp
+++ b/libwvdrmengine/mediacrypto/Android.bp
@@ -90,6 +90,8 @@ cc_library_static {
         "liblog",
     ],
 
+    cflags: ["-Wthread-safety"],
+
     proprietary: true,
 
 }
diff --git a/libwvdrmengine/mediacrypto/include_hidl/WVCryptoPlugin.h b/libwvdrmengine/mediacrypto/include_hidl/WVCryptoPlugin.h
index a4401f82..c4ee3529 100644
--- a/libwvdrmengine/mediacrypto/include_hidl/WVCryptoPlugin.h
+++ b/libwvdrmengine/mediacrypto/include_hidl/WVCryptoPlugin.h
@@ -7,8 +7,11 @@
 #ifndef WV_CRYPTO_PLUGIN_H_
 #define WV_CRYPTO_PLUGIN_H_
 
+#include <android-base/thread_annotations.h>
 #include <android/hidl/memory/1.0/IMemory.h>
 
+#include <mutex>
+
 #include "HidlTypes.h"
 #include "log.h"
 #include "wv_content_decryption_module.h"
@@ -60,7 +63,7 @@ struct WVCryptoPlugin : public ::drm::V1_4::ICryptoPlugin {
       const SharedBuffer& source,
       uint64_t offset,
       const DestinationBuffer& destination,
-      decrypt_1_2_cb _hidl_cb) override;
+      decrypt_1_2_cb _hidl_cb) override NO_THREAD_SAFETY_ANALYSIS; // use unique_lock
 
   Return<void> getLogMessages(
       getLogMessages_cb _hidl_cb) override;
@@ -73,7 +76,7 @@ struct WVCryptoPlugin : public ::drm::V1_4::ICryptoPlugin {
   wvcdm::LoggingUidSetter mLoggingUidSetter;
 
   wvcdm::CdmSessionId mSessionId;
-  std::map<uint32_t, sp<IMemory> > mSharedBufferMap;
+  std::map<uint32_t, sp<IMemory> > mSharedBufferMap GUARDED_BY(mSharedBufferLock);
 
   sp<wvcdm::WvContentDecryptionModule> const mCDM;
   uint32_t mUserId;
@@ -81,6 +84,8 @@ struct WVCryptoPlugin : public ::drm::V1_4::ICryptoPlugin {
   Status_V1_2 attemptDecrypt(
       const wvcdm::CdmDecryptionParametersV16& params,
       bool haveEncryptedSubsamples, std::string* errorDetailMsg);
+
+  std::mutex mSharedBufferLock;
 };
 
 }  // namespace widevine
diff --git a/libwvdrmengine/mediacrypto/src_hidl/WVCryptoPlugin.cpp b/libwvdrmengine/mediacrypto/src_hidl/WVCryptoPlugin.cpp
index 166b8e8f..b105aea9 100644
--- a/libwvdrmengine/mediacrypto/src_hidl/WVCryptoPlugin.cpp
+++ b/libwvdrmengine/mediacrypto/src_hidl/WVCryptoPlugin.cpp
@@ -121,6 +121,8 @@ Return<void> WVCryptoPlugin::setSharedBufferBase(
     const hidl_memory& base, uint32_t bufferId) {
   sp<IMemory> hidlMemory = mapMemory(base);
 
+  std::lock_guard<std::mutex> shared_buffer_lock(mSharedBufferLock);
+
   // allow mapMemory to return nullptr
   mSharedBufferMap[bufferId] = hidlMemory;
   return Void();
@@ -169,7 +171,7 @@ Return<void> WVCryptoPlugin::decrypt_1_2(
     uint64_t offset,
     const DestinationBuffer& destination,
     decrypt_1_2_cb _hidl_cb) {
-
+  std::unique_lock<std::mutex> lock(mSharedBufferLock);
   if (mSharedBufferMap.find(source.bufferId) == mSharedBufferMap.end()) {
     _hidl_cb(Status_V1_2::ERROR_DRM_CANNOT_HANDLE, 0,
              "source decrypt buffer base not set");
@@ -242,6 +244,9 @@ Return<void> WVCryptoPlugin::decrypt_1_2(
     destPtr = static_cast<void *>(handle);
   }
 
+  // release mSharedBufferLock
+  lock.unlock();
+
   // Set up the decrypt params
   CdmDecryptionParametersV16 params;
   params.key_id = cryptoKey;