From ce25b9d44c3dd19d07b84b5e913e8643e50700f2 Mon Sep 17 00:00:00 2001
From: Alex Dale <sigquit@google.com>
Date: Fri, 10 Mar 2023 15:50:55 -0800
Subject: [PATCH] Avoid null dereference with empty BCC strings.

[ Merge of http://go/wvgerrit/168482 ]

The function OEMCrypto_GetBootCertificateChain() does not always
provide an additional signature depending on the device.  However, the
CDM would still attempt to dereference the first character in the
additional signature buffer when empty.  This CL changes how the data
pointer to an output string is acquired.  Empty string will instead
pass in a null pointer.

Bug: 272643393
Test: run_prov40_tests
Test: atest GtsMediaTestCases
Change-Id: I10b0a3c7df4fc73272aa701bb01c60672645d4fc
(cherry picked from commit a878e7b98d233ba3552b3c0ba8434e81160a837b)
---
 .../cdm/core/src/crypto_session.cpp           | 31 +++++++++++--------
 1 file changed, 18 insertions(+), 13 deletions(-)

diff --git a/libwvdrmengine/cdm/core/src/crypto_session.cpp b/libwvdrmengine/cdm/core/src/crypto_session.cpp
index 25b46b11..6d9ca395 100644
--- a/libwvdrmengine/cdm/core/src/crypto_session.cpp
+++ b/libwvdrmengine/cdm/core/src/crypto_session.cpp
@@ -202,6 +202,12 @@ size_t GenericEncryptionBlockSize(CdmEncryptionAlgorithm algorithm) {
   }
   return kAes128BlockSize;
 }
+
+uint8_t* MutableStringDataPointer(std::string* s) {
+  if (s == nullptr) return nullptr;
+  if (s->empty()) return nullptr;
+  return reinterpret_cast<uint8_t*>(&s->front());
+}
 }  // namespace
 
 // CryptoSession variables allocation.
@@ -1385,18 +1391,18 @@ CdmResponseType CryptoSession::GetBootCertificateChain(
 
   size_t bcc_length = 0;
   size_t additional_signature_length = 0;
-  OEMCryptoResult sts;
-  WithOecReadLock("GetBootCertificateChain Attempt 1", [&] {
-    sts = OEMCrypto_GetBootCertificateChain(nullptr, &bcc_length, nullptr,
-                                            &additional_signature_length);
-  });
+  OEMCryptoResult sts =
+      WithOecReadLock("GetBootCertificateChain Attempt 1", [&] {
+        return OEMCrypto_GetBootCertificateChain(nullptr, &bcc_length, nullptr,
+                                                 &additional_signature_length);
+      });
   if (sts == OEMCrypto_ERROR_SHORT_BUFFER) {
     bcc->resize(bcc_length);
     additional_signature->resize(additional_signature_length);
-    WithOecReadLock("GetBootCertificateChain Attempt 2", [&] {
-      sts = OEMCrypto_GetBootCertificateChain(
-          reinterpret_cast<uint8_t*>(&bcc->front()), &bcc_length,
-          reinterpret_cast<uint8_t*>(&additional_signature->front()),
+    sts = WithOecReadLock("GetBootCertificateChain Attempt 2", [&] {
+      return OEMCrypto_GetBootCertificateChain(
+          MutableStringDataPointer(bcc), &bcc_length,
+          MutableStringDataPointer(additional_signature),
           &additional_signature_length);
     });
   }
@@ -1444,11 +1450,10 @@ CdmResponseType CryptoSession::GenerateCertificateKeyPair(
   WithOecSessionLock("GenerateCertificateKeyPair Attempt 2", [&] {
     M_TIME(
         status = OEMCrypto_GenerateCertificateKeyPair(
-            oec_session_id_, reinterpret_cast<uint8_t*>(&public_key->front()),
-            &public_key_length,
-            reinterpret_cast<uint8_t*>(&public_key_signature->front()),
+            oec_session_id_, MutableStringDataPointer(public_key),
+            &public_key_length, MutableStringDataPointer(public_key_signature),
             &public_key_signature_length,
-            reinterpret_cast<uint8_t*>(&wrapped_private_key->front()),
+            MutableStringDataPointer(wrapped_private_key),
             &wrapped_private_key_length, &oemcrypto_key_type),
         metrics_, oemcrypto_generate_certificate_key_pair_, status);
   });