From 5d6f6f98ace1275c2ebe0439df4c26592edefeaa Mon Sep 17 00:00:00 2001
From: Feras Aldahlawi <frs@chromium.org>
Date: Tue, 15 Sep 2026 10:06:07 -0700
Subject: [PATCH] Fix cross-profile identifier leak in EME Storage ID via OTR profile

In branded builds on Windows, macOS, and ChromeOS
(enable_cdm_storage_id), CdmDocumentServiceImpl::GetStorageId computes
the EME Storage ID from the salt returned by MediaStorageIdSalt.
Previously, GetStorageIdSaltFromProfile passed profile->GetPrefs()
directly to MediaStorageIdSalt::GetSalt().

For off-the-record profiles, the PrefService is backed by an
OverlayUserPrefStore over the parent regular profile's persistent store.
When reading media.storage_id_salt, the read falls through to the
regular profile's persisted salt if one exists. Because the EME Storage
ID is computed as SHA256(machine_id + origin + key + profile_salt), an
Incognito CDM received a Storage ID byte-identical to the one the same
origin receives in regular browsing mode.

Bug: 559776316
Change-Id: Ia75a20d2a9d351ebe6236b63c029a5696a6a6964
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8408727
Reviewed-by: Frank Liberato <liberato@chromium.org>
Commit-Queue: Feras Aldahlawi <frs@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1698212}
---
diff --git a/chrome/browser/media/cdm_document_service_impl.cc b/chrome/browser/media/cdm_document_service_impl.cc
index c2cdf30..019489e5 100644
--- a/chrome/browser/media/cdm_document_service_impl.cc
+++ b/chrome/browser/media/cdm_document_service_impl.cc
@@ -71,7 +71,7 @@
   DCHECK(rfh);
   Profile* profile =
       Profile::FromBrowserContext(rfh->GetProcess()->GetBrowserContext());
-  return MediaStorageIdSalt::GetSalt(profile->GetPrefs());
+  return MediaStorageIdSalt::GetSalt(profile);
 }
 
 #endif  // BUILDFLAG(ENABLE_CDM_STORAGE_ID)
diff --git a/chrome/browser/media/media_storage_id_salt.cc b/chrome/browser/media/media_storage_id_salt.cc
index eeca490..066ce36 100644
--- a/chrome/browser/media/media_storage_id_salt.cc
+++ b/chrome/browser/media/media_storage_id_salt.cc
@@ -5,37 +5,80 @@
 #include "chrome/browser/media/media_storage_id_salt.h"
 
 #include <string>
+#include <vector>
 
 #include "base/logging.h"
 #include "base/strings/string_number_conversions.h"
+#include "base/supports_user_data.h"
+#include "chrome/browser/profiles/profile.h"
 #include "chrome/common/pref_names.h"
 #include "components/prefs/pref_registry_simple.h"
 #include "components/prefs/pref_service.h"
 #include "crypto/random.h"
 
-std::vector<uint8_t> MediaStorageIdSalt::GetSalt(PrefService* pref_service) {
+namespace {
+
+struct MediaStorageIdSaltUserData : public base::SupportsUserData::Data {
+  explicit MediaStorageIdSaltUserData(std::vector<uint8_t> salt)
+      : salt(std::move(salt)) {}
+  std::vector<uint8_t> salt;
+};
+
+const void* const kMediaStorageIdSaltUserDataKey =
+    &kMediaStorageIdSaltUserDataKey;
+
+std::vector<uint8_t> GetSaltFromPrefs(PrefService* pref_service) {
   // Salt is stored as hex-encoded string.
   std::string encoded_salt =
       pref_service->GetString(prefs::kMediaStorageIdSalt);
   std::vector<uint8_t> salt;
-  if (encoded_salt.length() != kSaltLength * 2 ||
-      !base::HexStringToBytes(encoded_salt, &salt)) {
-    // If the salt is not the proper format log an error.
-    if (encoded_salt.length() > 0) {
-      DLOG(ERROR) << "Saved value for " << prefs::kMediaStorageIdSalt
-                  << " is not valid: " << encoded_salt;
-      // Continue on to generate a new one.
-    }
-
-    // If the salt doesn't exist, generate a new one.
-    salt = crypto::RandBytesAsVector(kSaltLength);
-    encoded_salt = base::HexEncode(salt);
-    pref_service->SetString(prefs::kMediaStorageIdSalt, encoded_salt);
+  if (encoded_salt.length() == MediaStorageIdSalt::kSaltLength * 2 &&
+      base::HexStringToBytes(encoded_salt, &salt)) {
+    return salt;
   }
 
+  if (!encoded_salt.empty()) {
+    DLOG(ERROR) << "Saved value for " << prefs::kMediaStorageIdSalt
+                << " is not valid: " << encoded_salt;
+  }
+
+  // Generate and store a new salt.
+  salt = crypto::RandBytesAsVector(MediaStorageIdSalt::kSaltLength);
+  pref_service->SetString(prefs::kMediaStorageIdSalt, base::HexEncode(salt));
   return salt;
 }
 
+std::vector<uint8_t> GetSaltForOffTheRecordProfile(Profile* profile) {
+  auto* user_data = static_cast<MediaStorageIdSaltUserData*>(
+      profile->GetUserData(kMediaStorageIdSaltUserDataKey));
+  if (user_data) {
+    return user_data->salt;
+  }
+
+  std::vector<uint8_t> salt =
+      crypto::RandBytesAsVector(MediaStorageIdSalt::kSaltLength);
+  profile->SetUserData(kMediaStorageIdSaltUserDataKey,
+                       std::make_unique<MediaStorageIdSaltUserData>(salt));
+  return salt;
+}
+
+}  // namespace
+
+std::vector<uint8_t> MediaStorageIdSalt::GetSalt(Profile* profile) {
+  if (!profile) {
+    return {};
+  }
+
+  // Regular profiles persist the salt in user preferences.
+  if (!profile->IsOffTheRecord()) {
+    return GetSaltFromPrefs(profile->GetPrefs());
+  }
+
+  // Off-the-record profiles use an in-memory ephemeral salt to prevent
+  // falling through to the regular profile's persistent pref store.
+  return GetSaltForOffTheRecordProfile(profile);
+}
+
 void MediaStorageIdSalt::RegisterProfilePrefs(PrefRegistrySimple* registry) {
   registry->RegisterStringPref(prefs::kMediaStorageIdSalt, std::string());
 }
diff --git a/chrome/browser/media/media_storage_id_salt.h b/chrome/browser/media/media_storage_id_salt.h
index 00cd6bc..16159a2 100644
--- a/chrome/browser/media/media_storage_id_salt.h
+++ b/chrome/browser/media/media_storage_id_salt.h
@@ -10,9 +10,9 @@
 #include <vector>
 
 class PrefRegistrySimple;
-class PrefService;
+class Profile;
 
-// MediaStorageIDSalt is responsible for creating and retrieving a salt string
+// MediaStorageIdSalt is responsible for creating and retrieving a salt string
 // that is used when creating Storage IDs.
 class MediaStorageIdSalt {
  public:
@@ -22,8 +22,10 @@
   MediaStorageIdSalt(const MediaStorageIdSalt&) = delete;
   MediaStorageIdSalt& operator=(const MediaStorageIdSalt&) = delete;
 
-  // Retrieves the current salt. If one does not currently exist it is created.
-  static std::vector<uint8_t> GetSalt(PrefService* pref_service);
+  // Retrieves the current salt for `profile`. If one does not currently exist
+  // it is created. For off-the-record profiles, an ephemeral in-memory salt is
+  // used.
+  static std::vector<uint8_t> GetSalt(Profile* profile);
 
   static void RegisterProfilePrefs(PrefRegistrySimple* registry);
 };
diff --git a/chrome/browser/media/media_storage_id_salt_unittest.cc b/chrome/browser/media/media_storage_id_salt_unittest.cc
index 2dbd2bb1..7903bee67 100644
--- a/chrome/browser/media/media_storage_id_salt_unittest.cc
+++ b/chrome/browser/media/media_storage_id_salt_unittest.cc
@@ -4,51 +4,89 @@
 
 #include "chrome/browser/media/media_storage_id_salt.h"
 
+#include <string>
+#include <vector>
+
+#include "chrome/browser/profiles/profile.h"
 #include "chrome/common/pref_names.h"
+#include "chrome/test/base/testing_profile.h"
 #include "components/prefs/testing_pref_service.h"
+#include "content/public/test/browser_task_environment.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
-TEST(MediaStorageIdSalt, Register) {
+class MediaStorageIdSaltTest : public testing::Test {
+ protected:
+  content::BrowserTaskEnvironment task_environment_;
+  TestingProfile profile_;
+};
+
+TEST_F(MediaStorageIdSaltTest, Register) {
   TestingPrefServiceSimple prefs;
 
   MediaStorageIdSalt::RegisterProfilePrefs(prefs.registry());
 }
 
-TEST(MediaStorageIdSalt, Create) {
-  TestingPrefServiceSimple prefs;
-
-  MediaStorageIdSalt::RegisterProfilePrefs(prefs.registry());
-  std::vector<uint8_t> salt = MediaStorageIdSalt::GetSalt(&prefs);
+TEST_F(MediaStorageIdSaltTest, Create) {
+  std::vector<uint8_t> salt = MediaStorageIdSalt::GetSalt(&profile_);
   EXPECT_EQ(static_cast<size_t>(MediaStorageIdSalt::kSaltLength), salt.size());
 }
 
-TEST(MediaStorageIdSalt, Recreate) {
-  TestingPrefServiceSimple prefs;
-
-  MediaStorageIdSalt::RegisterProfilePrefs(prefs.registry());
-  std::vector<uint8_t> original_salt = MediaStorageIdSalt::GetSalt(&prefs);
+TEST_F(MediaStorageIdSaltTest, Recreate) {
+  std::vector<uint8_t> original_salt = MediaStorageIdSalt::GetSalt(&profile_);
   EXPECT_EQ(static_cast<size_t>(MediaStorageIdSalt::kSaltLength),
             original_salt.size());
 
   // Now that the salt is created, mess it up and then try fetching it again
   // (should generate a new salt and log an error).
-  prefs.SetString(prefs::kMediaStorageIdSalt, "123");
-  std::vector<uint8_t> new_salt = MediaStorageIdSalt::GetSalt(&prefs);
+  profile_.GetPrefs()->SetString(prefs::kMediaStorageIdSalt, "123");
+  std::vector<uint8_t> new_salt = MediaStorageIdSalt::GetSalt(&profile_);
   EXPECT_EQ(static_cast<size_t>(MediaStorageIdSalt::kSaltLength),
             new_salt.size());
   EXPECT_NE(original_salt, new_salt);
 }
 
-TEST(MediaStorageIdSalt, FetchTwice) {
-  TestingPrefServiceSimple prefs;
-
-  MediaStorageIdSalt::RegisterProfilePrefs(prefs.registry());
-  std::vector<uint8_t> salt1 = MediaStorageIdSalt::GetSalt(&prefs);
+TEST_F(MediaStorageIdSaltTest, FetchTwice) {
+  std::vector<uint8_t> salt1 = MediaStorageIdSalt::GetSalt(&profile_);
   EXPECT_EQ(static_cast<size_t>(MediaStorageIdSalt::kSaltLength), salt1.size());
 
   // Fetch the salt again. Should be the same value.
-  std::vector<uint8_t> salt2 = MediaStorageIdSalt::GetSalt(&prefs);
+  std::vector<uint8_t> salt2 = MediaStorageIdSalt::GetSalt(&profile_);
   EXPECT_EQ(static_cast<size_t>(MediaStorageIdSalt::kSaltLength), salt2.size());
   EXPECT_EQ(salt1, salt2);
 }
+
+TEST_F(MediaStorageIdSaltTest, OffTheRecord) {
+  std::vector<uint8_t> regular_salt = MediaStorageIdSalt::GetSalt(&profile_);
+  EXPECT_EQ(static_cast<size_t>(MediaStorageIdSalt::kSaltLength),
+            regular_salt.size());
+
+  TestingProfile* incognito_profile =
+      TestingProfile::Builder().BuildIncognito(&profile_);
+  std::vector<uint8_t> incognito_salt1 =
+      MediaStorageIdSalt::GetSalt(incognito_profile);
+  EXPECT_EQ(static_cast<size_t>(MediaStorageIdSalt::kSaltLength),
+            incognito_salt1.size());
+  EXPECT_NE(regular_salt, incognito_salt1);
+
+  // Calling again on the same Incognito profile should return the same salt.
+  std::vector<uint8_t> incognito_salt2 =
+      MediaStorageIdSalt::GetSalt(incognito_profile);
+  EXPECT_EQ(incognito_salt1, incognito_salt2);
+}
+
+TEST_F(MediaStorageIdSaltTest, IncognitoFirstDoesNotLeakToRegular) {
+  TestingProfile* incognito_profile =
+      TestingProfile::Builder().BuildIncognito(&profile_);
+  std::vector<uint8_t> incognito_salt =
+      MediaStorageIdSalt::GetSalt(incognito_profile);
+  EXPECT_EQ(static_cast<size_t>(MediaStorageIdSalt::kSaltLength),
+            incognito_salt.size());
+
+  // Fetching the regular profile's salt after Incognito should generate a
+  // distinct salt and not reuse or leak the Incognito salt.
+  std::vector<uint8_t> regular_salt = MediaStorageIdSalt::GetSalt(&profile_);
+  EXPECT_EQ(static_cast<size_t>(MediaStorageIdSalt::kSaltLength),
+            regular_salt.size());
+  EXPECT_NE(incognito_salt, regular_salt);
+}

diff --git a/chrome/browser/media/media_storage_id_salt_unittest.cc b/chrome/browser/media/media_storage_id_salt_unittest.cc
index 2dbd2bb1..7903bee67 100644
--- a/chrome/browser/media/media_storage_id_salt_unittest.cc
+++ b/chrome/browser/media/media_storage_id_salt_unittest.cc
@@ -4,51 +4,89 @@
 
 #include "chrome/browser/media/media_storage_id_salt.h"
 
+#include <string>
+#include <vector>
+
+#include "chrome/browser/profiles/profile.h"
 #include "chrome/common/pref_names.h"
+#include "chrome/test/base/testing_profile.h"
 #include "components/prefs/testing_pref_service.h"
+#include "content/public/test/browser_task_environment.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
-TEST(MediaStorageIdSalt, Register) {
+class MediaStorageIdSaltTest : public testing::Test {
+ protected:
+  content::BrowserTaskEnvironment task_environment_;
+  TestingProfile profile_;
+};
+
+TEST_F(MediaStorageIdSaltTest, Register) {
   TestingPrefServiceSimple prefs;
 
   MediaStorageIdSalt::RegisterProfilePrefs(prefs.registry());
 }
 
-TEST(MediaStorageIdSalt, Create) {
-  TestingPrefServiceSimple prefs;
-
-  MediaStorageIdSalt::RegisterProfilePrefs(prefs.registry());
-  std::vector<uint8_t> salt = MediaStorageIdSalt::GetSalt(&prefs);
+TEST_F(MediaStorageIdSaltTest, Create) {
+  std::vector<uint8_t> salt = MediaStorageIdSalt::GetSalt(&profile_);
   EXPECT_EQ(static_cast<size_t>(MediaStorageIdSalt::kSaltLength), salt.size());
 }
 
-TEST(MediaStorageIdSalt, Recreate) {
-  TestingPrefServiceSimple prefs;
-
-  MediaStorageIdSalt::RegisterProfilePrefs(prefs.registry());
-  std::vector<uint8_t> original_salt = MediaStorageIdSalt::GetSalt(&prefs);
+TEST_F(MediaStorageIdSaltTest, Recreate) {
+  std::vector<uint8_t> original_salt = MediaStorageIdSalt::GetSalt(&profile_);
   EXPECT_EQ(static_cast<size_t>(MediaStorageIdSalt::kSaltLength),
             original_salt.size());
 
   // Now that the salt is created, mess it up and then try fetching it again
   // (should generate a new salt and log an error).
-  prefs.SetString(prefs::kMediaStorageIdSalt, "123");
-  std::vector<uint8_t> new_salt = MediaStorageIdSalt::GetSalt(&prefs);
+  profile_.GetPrefs()->SetString(prefs::kMediaStorageIdSalt, "123");
+  std::vector<uint8_t> new_salt = MediaStorageIdSalt::GetSalt(&profile_);
   EXPECT_EQ(static_cast<size_t>(MediaStorageIdSalt::kSaltLength),
             new_salt.size());
   EXPECT_NE(original_salt, new_salt);
 }
 
-TEST(MediaStorageIdSalt, FetchTwice) {
-  TestingPrefServiceSimple prefs;
-
-  MediaStorageIdSalt::RegisterProfilePrefs(prefs.registry());
-  std::vector<uint8_t> salt1 = MediaStorageIdSalt::GetSalt(&prefs);
+TEST_F(MediaStorageIdSaltTest, FetchTwice) {
+  std::vector<uint8_t> salt1 = MediaStorageIdSalt::GetSalt(&profile_);
   EXPECT_EQ(static_cast<size_t>(MediaStorageIdSalt::kSaltLength), salt1.size());
 
   // Fetch the salt again. Should be the same value.
-  std::vector<uint8_t> salt2 = MediaStorageIdSalt::GetSalt(&prefs);
+  std::vector<uint8_t> salt2 = MediaStorageIdSalt::GetSalt(&profile_);
   EXPECT_EQ(static_cast<size_t>(MediaStorageIdSalt::kSaltLength), salt2.size());
   EXPECT_EQ(salt1, salt2);
 }
+
+TEST_F(MediaStorageIdSaltTest, OffTheRecord) {
+  std::vector<uint8_t> regular_salt = MediaStorageIdSalt::GetSalt(&profile_);
+  EXPECT_EQ(static_cast<size_t>(MediaStorageIdSalt::kSaltLength),
+            regular_salt.size());
+
+  TestingProfile* incognito_profile =
+      TestingProfile::Builder().BuildIncognito(&profile_);
+  std::vector<uint8_t> incognito_salt1 =
+      MediaStorageIdSalt::GetSalt(incognito_profile);
+  EXPECT_EQ(static_cast<size_t>(MediaStorageIdSalt::kSaltLength),
+            incognito_salt1.size());
+  EXPECT_NE(regular_salt, incognito_salt1);
+
+  // Calling again on the same Incognito profile should return the same salt.
+  std::vector<uint8_t> incognito_salt2 =
+      MediaStorageIdSalt::GetSalt(incognito_profile);
+  EXPECT_EQ(incognito_salt1, incognito_salt2);
+}
+
+TEST_F(MediaStorageIdSaltTest, IncognitoFirstDoesNotLeakToRegular) {
+  TestingProfile* incognito_profile =
+      TestingProfile::Builder().BuildIncognito(&profile_);
+  std::vector<uint8_t> incognito_salt =
+      MediaStorageIdSalt::GetSalt(incognito_profile);
+  EXPECT_EQ(static_cast<size_t>(MediaStorageIdSalt::kSaltLength),
+            incognito_salt.size());
+
+  // Fetching the regular profile's salt after Incognito should generate a
+  // distinct salt and not reuse or leak the Incognito salt.
+  std::vector<uint8_t> regular_salt = MediaStorageIdSalt::GetSalt(&profile_);
+  EXPECT_EQ(static_cast<size_t>(MediaStorageIdSalt::kSaltLength),
+            regular_salt.size());
+  EXPECT_NE(incognito_salt, regular_salt);
+}