Fix UAF in clearkey service's MemoryFileSystem Fixes the potential issue of a UAF(Used After Free) of a MemoryFileSystem handle in clearkey hidl service. The issue can occur in a race codition where the handle can be freed twice. Test: sts-tradefed sts-tradefed run sts-engbuild-no-spl-lock -m StsHostTestCases --test android.security.sts.Bug_176237595#testPocBug_176237595 Test: push to device with target_hwasan-userdebug build adb shell /data/local/tmp/Bug-176237595_sts64 Bug: 176237595 Change-Id: I024f6716f66b21849121bef8780170014f4bf2b9
diff --git a/drm/mediadrm/plugins/clearkey/hidl/Android.bp b/drm/mediadrm/plugins/clearkey/hidl/Android.bp index a194416..c6afa60 100644 --- a/drm/mediadrm/plugins/clearkey/hidl/Android.bp +++ b/drm/mediadrm/plugins/clearkey/hidl/Android.bp
@@ -37,7 +37,7 @@ relative_install_path: "hw", - cflags: ["-Wall", "-Werror"], + cflags: ["-Wall", "-Werror", "-Wthread-safety"], shared_libs: [ "android.hardware.drm@1.0",
diff --git a/drm/mediadrm/plugins/clearkey/hidl/DrmPlugin.cpp b/drm/mediadrm/plugins/clearkey/hidl/DrmPlugin.cpp index a77759e..c5ceee9 100644 --- a/drm/mediadrm/plugins/clearkey/hidl/DrmPlugin.cpp +++ b/drm/mediadrm/plugins/clearkey/hidl/DrmPlugin.cpp
@@ -220,6 +220,7 @@ if (requestString.find(kOfflineLicense) != std::string::npos) { std::string emptyResponse; std::string keySetIdString(keySetId.begin(), keySetId.end()); + Mutex::Autolock lock(mFileHandleLock); if (!mFileHandle.StoreLicense(keySetIdString, DeviceFiles::kLicenseStateReleasing, emptyResponse)) { @@ -335,6 +336,7 @@ } *keySetId = kKeySetIdPrefix + ByteArrayToHexString( reinterpret_cast<const uint8_t*>(randomData.data()), randomData.size()); + Mutex::Autolock lock(mFileHandleLock); if (mFileHandle.LicenseExists(*keySetId)) { // collision, regenerate ALOGV("Retry generating KeySetId"); @@ -392,6 +394,7 @@ if (status == Status::OK) { if (isOfflineLicense) { if (isRelease) { + Mutex::Autolock lock(mFileHandleLock); mFileHandle.DeleteLicense(keySetId); mSessionLibrary->destroySession(session); } else { @@ -400,6 +403,7 @@ return Void(); } + Mutex::Autolock lock(mFileHandleLock); bool ok = mFileHandle.StoreLicense( keySetId, DeviceFiles::kLicenseStateActive, @@ -454,6 +458,7 @@ DeviceFiles::LicenseState licenseState; std::string offlineLicense; Status status = Status::OK; + Mutex::Autolock lock(mFileHandleLock); if (!mFileHandle.RetrieveLicense(std::string(keySetId.begin(), keySetId.end()), &licenseState, &offlineLicense)) { ALOGE("Failed to restore offline license"); @@ -704,6 +709,8 @@ } Return<void> DrmPlugin::getOfflineLicenseKeySetIds(getOfflineLicenseKeySetIds_cb _hidl_cb) { + Mutex::Autolock lock(mFileHandleLock); + std::vector<std::string> licenseNames = mFileHandle.ListLicenses(); std::vector<KeySetId> keySetIds; if (mMockError != Status_V1_2::OK) { @@ -724,6 +731,7 @@ return toStatus_1_0(mMockError); } std::string licenseName(keySetId.begin(), keySetId.end()); + Mutex::Autolock lock(mFileHandleLock); if (mFileHandle.DeleteLicense(licenseName)) { return Status::OK; } @@ -732,6 +740,8 @@ Return<void> DrmPlugin::getOfflineLicenseState(const KeySetId& keySetId, getOfflineLicenseState_cb _hidl_cb) { + Mutex::Autolock lock(mFileHandleLock); + std::string licenseName(keySetId.begin(), keySetId.end()); DeviceFiles::LicenseState state; std::string license;
diff --git a/drm/mediadrm/plugins/clearkey/hidl/MemoryFileSystem.cpp b/drm/mediadrm/plugins/clearkey/hidl/MemoryFileSystem.cpp index 2dcd00f..d29acac 100644 --- a/drm/mediadrm/plugins/clearkey/hidl/MemoryFileSystem.cpp +++ b/drm/mediadrm/plugins/clearkey/hidl/MemoryFileSystem.cpp
@@ -24,11 +24,13 @@ } bool MemoryFileSystem::FileExists(const std::string& fileName) const { + std::lock_guard<std::mutex> lock(mMemoryFileSystemLock); auto result = mMemoryFileSystem.find(fileName); return result != mMemoryFileSystem.end(); } ssize_t MemoryFileSystem::GetFileSize(const std::string& fileName) const { + std::lock_guard<std::mutex> lock(mMemoryFileSystemLock); auto result = mMemoryFileSystem.find(fileName); if (result != mMemoryFileSystem.end()) { return static_cast<ssize_t>(result->second.getFileSize()); @@ -40,6 +42,7 @@ std::vector<std::string> MemoryFileSystem::ListFiles() const { std::vector<std::string> list; + std::lock_guard<std::mutex> lock(mMemoryFileSystemLock); for (const auto& filename : mMemoryFileSystem) { list.push_back(filename.first); } @@ -48,6 +51,7 @@ size_t MemoryFileSystem::Read(const std::string& path, std::string* buffer) { std::string key = GetFileName(path); + std::lock_guard<std::mutex> lock(mMemoryFileSystemLock); auto result = mMemoryFileSystem.find(key); if (result != mMemoryFileSystem.end()) { std::string serializedHashFile = result->second.getContent(); @@ -61,6 +65,7 @@ size_t MemoryFileSystem::Write(const std::string& path, const MemoryFile& memoryFile) { std::string key = GetFileName(path); + std::lock_guard<std::mutex> lock(mMemoryFileSystemLock); auto result = mMemoryFileSystem.find(key); if (result != mMemoryFileSystem.end()) { mMemoryFileSystem.erase(key); @@ -70,6 +75,7 @@ } bool MemoryFileSystem::RemoveFile(const std::string& fileName) { + std::lock_guard<std::mutex> lock(mMemoryFileSystemLock); auto result = mMemoryFileSystem.find(fileName); if (result != mMemoryFileSystem.end()) { mMemoryFileSystem.erase(result); @@ -81,6 +87,7 @@ } bool MemoryFileSystem::RemoveAllFiles() { + std::lock_guard<std::mutex> lock(mMemoryFileSystemLock); mMemoryFileSystem.clear(); return mMemoryFileSystem.empty(); }
diff --git a/drm/mediadrm/plugins/clearkey/hidl/include/DrmPlugin.h b/drm/mediadrm/plugins/clearkey/hidl/include/DrmPlugin.h index 076beb8..894985b 100644 --- a/drm/mediadrm/plugins/clearkey/hidl/include/DrmPlugin.h +++ b/drm/mediadrm/plugins/clearkey/hidl/include/DrmPlugin.h
@@ -416,7 +416,8 @@ mMockError = Status_V1_2::OK; } - DeviceFiles mFileHandle; + DeviceFiles mFileHandle GUARDED_BY(mFileHandleLock); + Mutex mFileHandleLock; Mutex mSecureStopLock; CLEARKEY_DISALLOW_COPY_AND_ASSIGN_AND_NEW(DrmPlugin);
diff --git a/drm/mediadrm/plugins/clearkey/hidl/include/MemoryFileSystem.h b/drm/mediadrm/plugins/clearkey/hidl/include/MemoryFileSystem.h index bcd9fd6..6ac0e2c 100644 --- a/drm/mediadrm/plugins/clearkey/hidl/include/MemoryFileSystem.h +++ b/drm/mediadrm/plugins/clearkey/hidl/include/MemoryFileSystem.h
@@ -5,7 +5,9 @@ #ifndef CLEARKEY_MEMORY_FILE_SYSTEM_H_ #define CLEARKEY_MEMORY_FILE_SYSTEM_H_ +#include <android-base/thread_annotations.h> #include <map> +#include <mutex> #include <string> #include "ClearKeyTypes.h" @@ -49,10 +51,12 @@ size_t Write(const std::string& pathName, const MemoryFile& memoryFile); private: + mutable std::mutex mMemoryFileSystemLock; + // License file name is made up of a unique keySetId, therefore, // the filename can be used as the key to locate licenses in the // memory file system. - std::map<std::string, MemoryFile> mMemoryFileSystem; + std::map<std::string, MemoryFile> mMemoryFileSystem GUARDED_BY(mMemoryFileSystemLock); std::string GetFileName(const std::string& path);