From a35d6c8e329143c4c2a72327b18b798b748aae5c Mon Sep 17 00:00:00 2001
From: Ytai Ben-Tsvi <ytai@google.com>
Date: Mon, 9 Sep 2019 13:23:14 -0700
Subject: [PATCH] Improve visibility of IMemory security risks

This change renames the IMemory raw pointer accessors to
unsecure*() to make it apparent to coders and code reviewers
that the returned buffer may potentially be shared with
untrusted processes, who may, after the fact, attempt to
read and/or modify the contents. This may lead to hard to
find security bugs and hopefully the rename makes it harder
to forget.

The change also attempts to fix all the callsites to make
everything build correctly, but in the processes, wherever the
callsite code was not obviously secure, I added a TODO requesting
the owners to either document why it's secure or to change the
code. Apologies in advance to the owners if there are some false
positives here - I don't have enough context to reason about all
the different callsites.

Test: Completely syntactic change. Made sure code still builds.
Change-Id: I13466e045f587e612ad80e518fee27f4da0bde5e
---
 .../mediacrypto/test/WVCryptoPlugin_test.cpp     | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/libwvdrmengine/mediacrypto/test/WVCryptoPlugin_test.cpp b/libwvdrmengine/mediacrypto/test/WVCryptoPlugin_test.cpp
index 7097a48b..abacd0fd 100644
--- a/libwvdrmengine/mediacrypto/test/WVCryptoPlugin_test.cpp
+++ b/libwvdrmengine/mediacrypto/test/WVCryptoPlugin_test.cpp
@@ -290,14 +290,14 @@ TEST_F(WVCryptoPluginTest, AttemptsToDecrypt) {
   sp<android::IMemory> source = memDealer->allocate(kDataSize);
   ASSERT_NE(source, nullptr);
   pSrc = static_cast<uint8_t*>(
-      static_cast<void*>(source->pointer()));
+      static_cast<void*>(source->unsecurePointer()));
   ASSERT_NE(pSrc, nullptr);
   memcpy(pSrc, in, source->size());
 
   sp<android::IMemory> destination = memDealer->allocate(kDataSize);
   ASSERT_NE(destination, nullptr);
   pDest = static_cast<uint8_t*>(
-      static_cast<void*>(destination->pointer()));
+      static_cast<void*>(destination->unsecurePointer()));
   ASSERT_NE(pDest, nullptr);
 
   uint8_t iv[5][KEY_IV_SIZE];
@@ -451,14 +451,14 @@ TEST_F(WVCryptoPluginTest, CommunicatesSecureBufferRequest) {
   sp<android::IMemory> source = memDealer->allocate(kDataSize);
   ASSERT_NE(source, nullptr);
   pSrc = static_cast<uint8_t*>(
-      static_cast<void*>(source->pointer()));
+      static_cast<void*>(source->unsecurePointer()));
   ASSERT_NE(pSrc, nullptr);
   memcpy(pSrc, in, source->size());
 
   sp<android::IMemory> destination = memDealer->allocate(kDataSize);
   ASSERT_NE(destination, nullptr);
   pDest = static_cast<uint8_t*>(
-      static_cast<void*>(destination->pointer()));
+      static_cast<void*>(destination->unsecurePointer()));
   ASSERT_NE(pDest, nullptr);
 
   WVCryptoPlugin plugin(sessionId, kSessionIdSize, cdm.get());
@@ -565,14 +565,14 @@ TEST_F(WVCryptoPluginTest, SetsFlagsForMinimumSubsampleRuns) {
   sp<android::IMemory> source = memDealer->allocate(kDataSize);
   ASSERT_NE(source, nullptr);
   pSrc = static_cast<uint8_t*>(
-      static_cast<void*>(source->pointer()));
+      static_cast<void*>(source->unsecurePointer()));
   ASSERT_NE(pSrc, nullptr);
   memcpy(pSrc, in, source->size());
 
   sp<android::IMemory> destination = memDealer->allocate(kDataSize);
   ASSERT_NE(destination, nullptr);
   pDest = static_cast<uint8_t*>(
-      static_cast<void*>(destination->pointer()));
+      static_cast<void*>(destination->unsecurePointer()));
   ASSERT_NE(pDest, nullptr);
 
   WVCryptoPlugin plugin(sessionId, kSessionIdSize, cdm.get());
@@ -678,14 +678,14 @@ TEST_F(WVCryptoPluginTest, AllowsSessionIdChanges) {
   sp<android::IMemory> source = memDealer->allocate(kDataSize);
   ASSERT_NE(source, nullptr);
   pSrc = static_cast<uint8_t*>(
-      static_cast<void*>(source->pointer()));
+      static_cast<void*>(source->unsecurePointer()));
   ASSERT_NE(pSrc, nullptr);
   memcpy(pSrc, in, source->size());
 
   sp<android::IMemory> destination = memDealer->allocate(kDataSize);
   ASSERT_NE(destination, nullptr);
   pDest = static_cast<uint8_t*>(
-      static_cast<void*>(destination->pointer()));
+      static_cast<void*>(destination->unsecurePointer()));
   ASSERT_NE(pDest, nullptr);
 
   uint8_t blank[1];  // Some compilers will not accept 0.