From 9b7a9fcfa3051d116a19f25368fccd4a623fe72d Mon Sep 17 00:00:00 2001
From: Edwin Wong <edwinwong@google.com>
Date: Wed, 25 Sep 2019 12:15:37 -0700
Subject: [DO NOT MERGE] Fix Heap use after free in clearkey getSecureStops

Security Vulnerability fix: Heap use after free in getSecureStops
in android.hardware.drm@1.1-service.clearkey

Test: adb shell ps | grep clearkey
  pid ID does not change after running drmpoc

Test: sts
  ANDROID_BUILD_TOP= ./android-sts/tools/sts-tradefed run sts-engbuild-no-spl-lock -m StsHostTestCases --test android.security.sts.Poc19_07#testPocBug_137878930

bug: 137878930
Merged-In: I78b2dc2bccde238a06398b3733cea8e574ea8ee7
Change-Id: I4fa2f0ad3f3360812987223f507a7394357080b3
---
 drm/mediadrm/plugins/clearkey/hidl/DrmPlugin.cpp    | 21 ++++++++++++++++++---
 .../plugins/clearkey/hidl/include/DrmPlugin.h       |  1 +
 2 files changed, 19 insertions(+), 3 deletions(-)

(limited to 'drm')

diff --git a/drm/mediadrm/plugins/clearkey/hidl/DrmPlugin.cpp b/drm/mediadrm/plugins/clearkey/hidl/DrmPlugin.cpp
index aab475ed88..546eb3eba9 100644
--- a/drm/mediadrm/plugins/clearkey/hidl/DrmPlugin.cpp
+++ b/drm/mediadrm/plugins/clearkey/hidl/DrmPlugin.cpp
@@ -111,6 +111,8 @@ void DrmPlugin::initProperties() {
 // The content in this secure stop is implementation dependent, the clearkey
 // secureStop does not serve as a reference implementation.
 void DrmPlugin::installSecureStop(const hidl_vec<uint8_t>& sessionId) {
+    Mutex::Autolock lock(mSecureStopLock);
+
     ClearkeySecureStop clearkeySecureStop;
     clearkeySecureStop.id = uint32ToVector(++mNextSecureStopId);
     clearkeySecureStop.data.assign(sessionId.begin(), sessionId.end());
@@ -744,6 +746,7 @@ Return<void> DrmPlugin::getOfflineLicenseState(const KeySetId& keySetId,
 }
 
 Return<void> DrmPlugin::getSecureStops(getSecureStops_cb _hidl_cb) {
+    mSecureStopLock.lock();
     std::vector<SecureStop> stops;
     for (auto itr = mSecureStops.begin(); itr != mSecureStops.end(); ++itr) {
         ClearkeySecureStop clearkeyStop = itr->second;
@@ -755,26 +758,32 @@ Return<void> DrmPlugin::getSecureStops(getSecureStops_cb _hidl_cb) {
         stop.opaqueData = toHidlVec(stopVec);
         stops.push_back(stop);
     }
+    mSecureStopLock.unlock();
+
     _hidl_cb(Status::OK, stops);
     return Void();
 }
 
 Return<void> DrmPlugin::getSecureStop(const hidl_vec<uint8_t>& secureStopId,
         getSecureStop_cb _hidl_cb) {
-    SecureStop stop;
+    std::vector<uint8_t> stopVec;
+
+    mSecureStopLock.lock();
     auto itr = mSecureStops.find(toVector(secureStopId));
     if (itr != mSecureStops.end()) {
         ClearkeySecureStop clearkeyStop = itr->second;
-        std::vector<uint8_t> stopVec;
         stopVec.insert(stopVec.end(), clearkeyStop.id.begin(), clearkeyStop.id.end());
         stopVec.insert(stopVec.end(), clearkeyStop.data.begin(), clearkeyStop.data.end());
+    }
+    mSecureStopLock.unlock();
 
+    SecureStop stop;
+    if (!stopVec.empty()) {
         stop.opaqueData = toHidlVec(stopVec);
         _hidl_cb(Status::OK, stop);
     } else {
         _hidl_cb(Status::BAD_VALUE, stop);
     }
-
     return Void();
 }
 
@@ -787,10 +796,12 @@ Return<Status> DrmPlugin::releaseAllSecureStops() {
 }
 
 Return<void> DrmPlugin::getSecureStopIds(getSecureStopIds_cb _hidl_cb) {
+    mSecureStopLock.lock();
     std::vector<SecureStopId> ids;
     for (auto itr = mSecureStops.begin(); itr != mSecureStops.end(); ++itr) {
         ids.push_back(itr->first);
     }
+    mSecureStopLock.unlock();
 
     _hidl_cb(Status::OK, toHidlVec(ids));
     return Void();
@@ -856,6 +867,8 @@ Return<Status> DrmPlugin::releaseSecureStops(const SecureStopRelease& ssRelease)
 }
 
 Return<Status> DrmPlugin::removeSecureStop(const hidl_vec<uint8_t>& secureStopId) {
+    Mutex::Autolock lock(mSecureStopLock);
+
     if (1 != mSecureStops.erase(toVector(secureStopId))) {
         return Status::BAD_VALUE;
     }
@@ -863,6 +876,8 @@ Return<Status> DrmPlugin::removeSecureStop(const hidl_vec<uint8_t>& secureStopId
 }
 
 Return<Status> DrmPlugin::removeAllSecureStops() {
+    Mutex::Autolock lock(mSecureStopLock);
+
     mSecureStops.clear();
     mNextSecureStopId = kSecureStopIdStart;
     return Status::OK;
diff --git a/drm/mediadrm/plugins/clearkey/hidl/include/DrmPlugin.h b/drm/mediadrm/plugins/clearkey/hidl/include/DrmPlugin.h
index f294d4d736..3de758945b 100644
--- a/drm/mediadrm/plugins/clearkey/hidl/include/DrmPlugin.h
+++ b/drm/mediadrm/plugins/clearkey/hidl/include/DrmPlugin.h
@@ -416,6 +416,7 @@ private:
     }
 
     DeviceFiles mFileHandle;
+    Mutex mSecureStopLock;
 
     CLEARKEY_DISALLOW_COPY_AND_ASSIGN_AND_NEW(DrmPlugin);
 };
-- 
cgit v1.2.3