From 116a72c15f0880be23af530d0967bbc6930ad91d Mon Sep 17 00:00:00 2001
From: Sangbaek Park <sangbaekpark@google.com>
Date: Wed, 27 May 2026 18:02:54 -0700
Subject: [PATCH] media: Fix UAF in MediaFoundationProtectionManager

MediaFoundationProtectionManager lacks a custom Release()
implementation. This allows it to be destructed on an asynchronous
Media Foundation threadpool thread rather than its bound sequence. This
leads to the potential cross-thread destruction of sequence-affine
members such as base::CancelableOnceClosure and base::WeakPtrFactory,
resulting in a race condition with pending tasks on the main task
runner and a Use-After-Free (UAF).

This CL implements a custom Release() override to safely route the
destruction to the designated task_runner_ via DeleteSoon().

Additionally, this CL:
* Adds MEDIA_EXPORT to MediaFoundationProtectionManager so it can be
  safely used inside unit tests during component builds.
* Adds a unit test that successfully reproduces the issue. It tracks
  the destruction sequence using a custom DestructionObserver bound
  to the WaitingCB. The test verifies that destruction occurs on
  the correct sequence, failing without the fix and passing with it.

Tests: { MediaFoundationProtectionManagerTest.DestructionOnTaskRunner }

Bug: 517004487
Change-Id: I0050d9509d8ed130e69eaa9cdfaa5a615228937c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7879769
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Commit-Queue: Sangbaek Park <sangbaekpark@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1637367}
---

diff --git a/media/renderers/BUILD.gn b/media/renderers/BUILD.gn
index 74361be..1dca83c 100644
--- a/media/renderers/BUILD.gn
+++ b/media/renderers/BUILD.gn
@@ -151,6 +151,7 @@
 
   if (is_win) {
     sources += [
+      "win/media_foundation_protection_manager_unittest.cc",
       "win/media_foundation_renderer_integration_test.cc",
       "win/media_foundation_renderer_unittest.cc",
       "win/media_foundation_source_wrapper_unittest.cc",
diff --git a/media/renderers/win/media_foundation_protection_manager.cc b/media/renderers/win/media_foundation_protection_manager.cc
index 7ba50a6c..8ccdc29 100644
--- a/media/renderers/win/media_foundation_protection_manager.cc
+++ b/media/renderers/win/media_foundation_protection_manager.cc
@@ -45,6 +45,18 @@
   return S_OK;
 }
 
+IFACEMETHODIMP_(ULONG) MediaFoundationProtectionManager::Release() {
+  ULONG ref_count = InternalRelease();
+  if (ref_count == 0) {
+    if (task_runner_ && !task_runner_->RunsTasksInCurrentSequence()) {
+      task_runner_->DeleteSoon(FROM_HERE, this);
+    } else {
+      delete this;
+    }
+  }
+  return ref_count;
+}
+
 HRESULT MediaFoundationProtectionManager::SetCdmProxy(
     scoped_refptr<MediaFoundationCdmProxy> cdm_proxy) {
   DVLOG_FUNC(1);
diff --git a/media/renderers/win/media_foundation_protection_manager.h b/media/renderers/win/media_foundation_protection_manager.h
index aad7fff..7df51f0 100644
--- a/media/renderers/win/media_foundation_protection_manager.h
+++ b/media/renderers/win/media_foundation_protection_manager.h
@@ -14,6 +14,7 @@
 #include "base/memory/scoped_refptr.h"
 #include "base/memory/weak_ptr.h"
 #include "base/task/sequenced_task_runner.h"
+#include "media/base/media_export.h"
 #include "media/base/waiting.h"
 #include "media/base/win/media_foundation_cdm_proxy.h"
 
@@ -26,7 +27,7 @@
 // required by IMFMediaEngineProtectedContent::SetContentProtectionManager in
 // https://docs.microsoft.com/en-us/windows/win32/api/mfmediaengine/nf-mfmediaengine-imfmediaengineprotectedcontent-setcontentprotectionmanager.
 //
-class MediaFoundationProtectionManager
+class MEDIA_EXPORT MediaFoundationProtectionManager
     : public Microsoft::WRL::RuntimeClass<
           Microsoft::WRL::RuntimeClassFlags<
               Microsoft::WRL::RuntimeClassType::WinRtClassicComMix |
@@ -42,6 +43,9 @@
       WaitingCB waiting_cb);
   HRESULT SetCdmProxy(scoped_refptr<MediaFoundationCdmProxy> cdm_proxy);
 
+  // IUnknown.
+  IFACEMETHODIMP_(ULONG) Release() override;
+
   // IMFContentProtectionManager.
   IFACEMETHODIMP BeginEnableContent(IMFActivate* enabler_activate,
                                     IMFTopology* topology,
diff --git a/media/renderers/win/media_foundation_protection_manager_unittest.cc b/media/renderers/win/media_foundation_protection_manager_unittest.cc
new file mode 100644
index 0000000..9dd8e07b
--- /dev/null
+++ b/media/renderers/win/media_foundation_protection_manager_unittest.cc
@@ -0,0 +1,98 @@
+// Copyright 2026 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "media/renderers/win/media_foundation_protection_manager.h"
+
+#include <wrl/client.h>
+
+#include "base/functional/bind.h"
+#include "base/memory/raw_ptr.h"
+#include "base/memory/ref_counted.h"
+#include "base/memory/scoped_refptr.h"
+#include "base/synchronization/waitable_event.h"
+#include "base/task/sequenced_task_runner.h"
+#include "base/task/thread_pool.h"
+#include "base/test/task_environment.h"
+#include "base/win/scoped_com_initializer.h"
+#include "media/base/waiting.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace media {
+
+namespace {
+
+class DestructionObserver
+    : public base::RefCountedThreadSafe<DestructionObserver> {
+ public:
+  DestructionObserver(scoped_refptr<base::SequencedTaskRunner> task_runner,
+                      bool* destroyed_on_correct_sequence)
+      : task_runner_(std::move(task_runner)),
+        destroyed_on_correct_sequence_(destroyed_on_correct_sequence) {}
+
+ private:
+  friend class base::RefCountedThreadSafe<DestructionObserver>;
+  ~DestructionObserver() {
+    *destroyed_on_correct_sequence_ =
+        task_runner_->RunsTasksInCurrentSequence();
+  }
+
+  scoped_refptr<base::SequencedTaskRunner> task_runner_;
+  raw_ptr<bool> destroyed_on_correct_sequence_;
+};
+
+}  // namespace
+
+class MediaFoundationProtectionManagerTest : public testing::Test {
+ public:
+  MediaFoundationProtectionManagerTest() = default;
+  ~MediaFoundationProtectionManagerTest() override = default;
+
+ protected:
+  base::test::TaskEnvironment task_environment_;
+  base::win::ScopedCOMInitializer com_initializer_{
+      base::win::ScopedCOMInitializer::kMTA};
+};
+
+TEST_F(MediaFoundationProtectionManagerTest, DestructionOnTaskRunner) {
+  scoped_refptr<base::SequencedTaskRunner> task_runner =
+      base::SequencedTaskRunner::GetCurrentDefault();
+
+  bool destroyed_on_correct_sequence = false;
+  auto observer = base::MakeRefCounted<DestructionObserver>(
+      task_runner, &destroyed_on_correct_sequence);
+
+  Microsoft::WRL::ComPtr<MediaFoundationProtectionManager> protection_manager;
+  HRESULT hr =
+      Microsoft::WRL::MakeAndInitialize<MediaFoundationProtectionManager>(
+          &protection_manager, task_runner,
+          base::BindRepeating(
+              [](scoped_refptr<DestructionObserver>, WaitingReason) {},
+              observer));
+  EXPECT_TRUE(SUCCEEDED(hr));
+
+  auto wrapper = protection_manager;
+  protection_manager.Reset();
+  observer.reset();
+
+  base::WaitableEvent event;
+  base::ThreadPool::PostTask(
+      FROM_HERE,
+      base::BindOnce(
+          [](Microsoft::WRL::ComPtr<MediaFoundationProtectionManager> wrapper,
+             base::WaitableEvent* event) {
+            wrapper.Reset();
+            event->Signal();
+          },
+          std::move(wrapper), &event));
+  event.Wait();
+
+  // Run the current thread's loop to execute the DeleteSoon task.
+  task_environment_.RunUntilIdle();
+
+  // This fails without the Use-After-Free fix for
+  // MediaFoundationProtectionManager.

diff --git a/media/renderers/win/media_foundation_protection_manager_unittest.cc b/media/renderers/win/media_foundation_protection_manager_unittest.cc
new file mode 100644
index 0000000..9dd8e07b
--- /dev/null
+++ b/media/renderers/win/media_foundation_protection_manager_unittest.cc
@@ -0,0 +1,98 @@
+// Copyright 2026 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "media/renderers/win/media_foundation_protection_manager.h"
+
+#include <wrl/client.h>
+
+#include "base/functional/bind.h"
+#include "base/memory/raw_ptr.h"
+#include "base/memory/ref_counted.h"
+#include "base/memory/scoped_refptr.h"
+#include "base/synchronization/waitable_event.h"
+#include "base/task/sequenced_task_runner.h"
+#include "base/task/thread_pool.h"
+#include "base/test/task_environment.h"
+#include "base/win/scoped_com_initializer.h"
+#include "media/base/waiting.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace media {
+
+namespace {
+
+class DestructionObserver
+    : public base::RefCountedThreadSafe<DestructionObserver> {
+ public:
+  DestructionObserver(scoped_refptr<base::SequencedTaskRunner> task_runner,
+                      bool* destroyed_on_correct_sequence)
+      : task_runner_(std::move(task_runner)),
+        destroyed_on_correct_sequence_(destroyed_on_correct_sequence) {}
+
+ private:
+  friend class base::RefCountedThreadSafe<DestructionObserver>;
+  ~DestructionObserver() {
+    *destroyed_on_correct_sequence_ =
+        task_runner_->RunsTasksInCurrentSequence();
+  }
+
+  scoped_refptr<base::SequencedTaskRunner> task_runner_;
+  raw_ptr<bool> destroyed_on_correct_sequence_;
+};
+
+}  // namespace
+
+class MediaFoundationProtectionManagerTest : public testing::Test {
+ public:
+  MediaFoundationProtectionManagerTest() = default;
+  ~MediaFoundationProtectionManagerTest() override = default;
+
+ protected:
+  base::test::TaskEnvironment task_environment_;
+  base::win::ScopedCOMInitializer com_initializer_{
+      base::win::ScopedCOMInitializer::kMTA};
+};
+
+TEST_F(MediaFoundationProtectionManagerTest, DestructionOnTaskRunner) {
+  scoped_refptr<base::SequencedTaskRunner> task_runner =
+      base::SequencedTaskRunner::GetCurrentDefault();
+
+  bool destroyed_on_correct_sequence = false;
+  auto observer = base::MakeRefCounted<DestructionObserver>(
+      task_runner, &destroyed_on_correct_sequence);
+
+  Microsoft::WRL::ComPtr<MediaFoundationProtectionManager> protection_manager;
+  HRESULT hr =
+      Microsoft::WRL::MakeAndInitialize<MediaFoundationProtectionManager>(
+          &protection_manager, task_runner,
+          base::BindRepeating(
+              [](scoped_refptr<DestructionObserver>, WaitingReason) {},
+              observer));
+  EXPECT_TRUE(SUCCEEDED(hr));
+
+  auto wrapper = protection_manager;
+  protection_manager.Reset();
+  observer.reset();
+
+  base::WaitableEvent event;
+  base::ThreadPool::PostTask(
+      FROM_HERE,
+      base::BindOnce(
+          [](Microsoft::WRL::ComPtr<MediaFoundationProtectionManager> wrapper,
+             base::WaitableEvent* event) {
+            wrapper.Reset();
+            event->Signal();
+          },
+          std::move(wrapper), &event));
+  event.Wait();
+
+  // Run the current thread's loop to execute the DeleteSoon task.
+  task_environment_.RunUntilIdle();
+
+  // This fails without the Use-After-Free fix for
+  // MediaFoundationProtectionManager.
+  EXPECT_TRUE(destroyed_on_correct_sequence);
+}
+
+}  // namespace media