CVE-2026-7350
Overview
Changed Functions
| Function | Change | Notes |
|---|---|---|
TEST_Fmedia/midi/midi_manager_unittest.cc |
modified | |
PlatformMidiManagerTestmedia/midi/midi_manager_unittest.cc |
modified |
Files Changed
media/midi/midi_manager.ccmedia/midi/midi_manager_unittest.ccmedia/midi/midi_manager_win.cc
Patch
From e16291b3e8885b72320f49feb86573ce598daaf6 Mon Sep 17 00:00:00 2001 From: Andrew Paseltiner <[email protected]> Date: Thu, 09 Apr 2026 06:07:33 -0700 Subject: [PATCH] media/midi: Fix data race and premature destruction in MIDI manager This change addresses a race condition in the Windows MIDI implementation where MidiManagerWin could be destroyed while initialization was still running on a background thread. Key improvements: 1. Updated MidiManager::HasOpenSession to check both clients_ and pending_clients_. This prevents MidiService from deleting the manager when a renderer closes its session before initialization completes, correctly reflecting that a background task is still active. 2. Synchronized ~MidiManagerWin by acquiring GetTaskLock() at the start of the destructor. This ensures that destruction is serialized with InitializeOnTaskRunner and other background tasks, preventing concurrent access to port vectors and pointers. 3. Added a regression test ReproduceLifecycleRace in midi_manager_unittest.cc to verify that HasOpenSession correctly accounts for pending clients. Fixed: 500018484 Change-Id: Ib7d5bc4deb5e00ac3d3f32ab561d7353d5699351 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7737022 Commit-Queue: Andrew Paseltiner <[email protected]> Reviewed-by: Takashi Toyoshima <[email protected]> Cr-Commit-Position: refs/heads/main@{#1612147} --- diff --git a/media/midi/midi_manager.cc b/media/midi/midi_manager.cc index 0c7c19a..5fca320 100644 --- a/media/midi/midi_manager.cc +++ b/media/midi/midi_manager.cc @@ -164,7 +164,7 @@ bool MidiManager::HasOpenSession() { base::AutoLock auto_lock(lock_); - return clients_.size() != 0u; + return !clients_.empty() || !pending_clients_.empty(); } void MidiManager::DispatchSendMidiData(MidiManagerClient* client, diff --git a/media/midi/midi_manager_unittest.cc b/media/midi/midi_manager_unittest.cc index 2be4a7b6..9eb48aa 100644 --- a/media/midi/midi_manager_unittest.cc +++ b/media/midi/midi_manager_unittest.cc @@ -219,6 +219,8 @@ base::WeakPtr<FakeMidiManagerFactory> factory() { return factory_; } + MidiService* service() { return service_.get(); } + private: base::test::TaskEnvironment env_; base::WeakPtr<FakeMidiManagerFactory> factory_; @@ -337,6 +339,31 @@ EXPECT_FALSE(test_future.IsReady()); } +TEST_F(MidiManagerTest, ReproduceLifecycleRace) { + base::test::TestFuture<void> test_future; + std::unique_ptr<FakeMidiManagerClient> client = + std::make_unique<FakeMidiManagerClient>(test_future.GetCallback()); + + // Start a session. This will put the client in pending_clients_. + StartSession(client.get()); + ASSERT_TRUE(factory()->manager()); + EXPECT_EQ(1U, factory()->manager()->GetPendingClientCount()); + EXPECT_EQ(0U, factory()->manager()->GetClientCount()); + + // FIXED: HasOpenSession() now checks both clients_ and pending_clients_, + // so it should return true while initialization is ongoing. + EXPECT_TRUE(factory()->manager()->HasOpenSession()); + + // If we end the session now, EndSession calls HasOpenSession to decide if it + // should delete the manager. Since the client is still in pending_clients_ + // until removed, EndSession correctly removes it. + EXPECT_TRUE(service()->EndSession(client.get())); + + // Now that the last client is gone (even from pending_clients_), + // HasOpenSession should return false and the manager should be deleted. + EXPECT_FALSE(factory()->manager()); +} + class PlatformMidiManagerTest : public ::testing::Test { public: PlatformMidiManagerTest() diff --git a/media/midi/midi_manager_win.cc b/media/midi/midi_manager_win.cc index d4d4550..2898226 100644 --- a/media/midi/midi_manager_win.cc +++ b/media/midi/midi_manager_win.cc @@ -723,6 +723,11 @@ if (instance_id_ == kInvalidInstanceId) return; + // Behind the lock below, we can safely access all members for finalization + // even on the I/O thread. This also ensures that no bound task runs on + // TaskRunner concurrently while destructing the instance. + base::AutoLock lock(*GetTaskLock()); + // Unregisters on the I/O thread. OnDevicesChanged() won't be called any more. CHECK(thread_runner_->BelongsToCurrentThread()); base::SystemMonitor::Get()->RemoveDevicesChangedObserver(this); @@ -738,20 +743,12 @@ // Invalidate instance bound tasks. { - base::AutoLock lock(*GetInstanceIdLock()); + base::AutoLock lock_id(*GetInstanceIdLock()); CHECK_EQ(instance_id_, g_active_instance_id); g_active_instance_id = kInvalidInstanceId; CHECK_EQ(this, g_manager_instance); g_manager_instance = nullptr; } - - // Ensures that no bound task runs on TaskRunner so to destruct the instance - // safely. - // Tasks that did not started yet will do nothing after invalidate the - // instance ID above. - // Behind the lock below, we can safely access all members for finalization - // even on the I/O thread. - base::AutoLock lock(*GetTaskLock()); } void MidiManagerWin::StartInitialization() {
Regression Test / PoC
diff --git a/media/midi/midi_manager_unittest.cc b/media/midi/midi_manager_unittest.cc
index 2be4a7b6..9eb48aa 100644
--- a/media/midi/midi_manager_unittest.cc
+++ b/media/midi/midi_manager_unittest.cc
@@ -219,6 +219,8 @@
base::WeakPtr<FakeMidiManagerFactory> factory() { return factory_; }
+ MidiService* service() { return service_.get(); }
+
private:
base::test::TaskEnvironment env_;
base::WeakPtr<FakeMidiManagerFactory> factory_;
@@ -337,6 +339,31 @@
EXPECT_FALSE(test_future.IsReady());
}
+TEST_F(MidiManagerTest, ReproduceLifecycleRace) {
+ base::test::TestFuture<void> test_future;
+ std::unique_ptr<FakeMidiManagerClient> client =
+ std::make_unique<FakeMidiManagerClient>(test_future.GetCallback());
+
+ // Start a session. This will put the client in pending_clients_.
+ StartSession(client.get());
+ ASSERT_TRUE(factory()->manager());
+ EXPECT_EQ(1U, factory()->manager()->GetPendingClientCount());
+ EXPECT_EQ(0U, factory()->manager()->GetClientCount());
+
+ // FIXED: HasOpenSession() now checks both clients_ and pending_clients_,
+ // so it should return true while initialization is ongoing.
+ EXPECT_TRUE(factory()->manager()->HasOpenSession());
+
+ // If we end the session now, EndSession calls HasOpenSession to decide if it
+ // should delete the manager. Since the client is still in pending_clients_
+ // until removed, EndSession correctly removes it.
+ EXPECT_TRUE(service()->EndSession(client.get()));
+
+ // Now that the last client is gone (even from pending_clients_),
+ // HasOpenSession should return false and the manager should be deleted.
+ EXPECT_FALSE(factory()->manager());
+}
+
class PlatformMidiManagerTest : public ::testing::Test {
public:
PlatformMidiManagerTest()
Original Bug Report
Potential Use-After-Free and Double-Free in MidiManagerWin via Race Condition
Project Fortify, an experimental security project, has identified the following potential security issue. If you’re a feature owner CC-ed on this bug, please do your best to review these reports without the security team.
Overview: A data race in the Windows MIDI implementation allows the MidiManagerWin destructor to run concurrently with its initialization task on a background thread. This unsynchronized access leads to potential heap corruption primitives in the browser process, including vector backing-store use-after-free and double-frees. A compromised renderer could potentially exploit this race to achieve a sandbox escape.
Affected files:
media/midi/midi_manager_win.ccmedia/midi/midi_manager.ccmedia/midi/midi_service.cccontent/browser/media/midi_host.cc
Estimated timestamp from git blame: 2018-10-03
Description
A race condition exists in the lifecycle management of MidiManagerWin that can lead to severe heap corruption in the browser process. The vulnerability stems from two interconnected logic flaws:
-
Premature Destruction: When a renderer requests a MIDI session,
MidiServicecreates aMidiManagerWinwhich begins hardware initialization on a background thread (MidiServiceThread). The client is added topending_clients_. If the renderer immediately closes its Mojo pipe,MidiHost::EndSessionhandles the disconnect and removes the client frompending_clients_. The service then callsMidiManager::HasOpenSession()to check if the manager should be destroyed. However,HasOpenSession()(inmedia/midi/midi_manager.cc) only checksclients_.size() != 0uand ignorespending_clients_. Because the count evaluates to 0,MidiServicedeletes the manager while the initialization task is actively running. -
Unsynchronized Destructor: The
~MidiManagerWin()destructor (inmedia/midi/midi_manager_win.cc) iterates overport_manager_->inputs()andoutputs()to callFinalize()on each port. Crucially, it does so without holdingGetTaskLock(). Concurrently, the background thread runningInitializeOnTaskRunnerholds the lock and invokesReflectActiveDeviceList, which pushes new ports into these very same vectors.
Impact
This race condition results in multiple highly exploitable memory corruption primitives in the unsandboxed browser process:
- Vector Use-After-Free: As the background thread pushes new ports, the
std::vectorbacking array can reallocate. The concurrent iteration in the IO thread’s destructor will then operate on a freed backing array. - Double-Free: The background thread calls
InPort::Open(), allocating a 32KB buffer into astd::unique_ptr(hdr_). Concurrently, the IO thread callsInPort::Finalize(), which moves that sameunique_ptr. This torn state causesMIDIHDRDeleterto double-free the heap chunk. - Map Corruption: Concurrent reads and writes to
hmidiin_to_index_map_corrupt the underlyingstd::mapRed-Black tree.
Because these structures are not protected by MiraclePtr, a compromised renderer could potentially groom the heap and weaponize these primitives to achieve a full Sandbox Escape (Browser Process RCE).
Potential Steps to Trigger
(Note: Our tooling does not execute code; these are suggested steps an attacker would follow based on static analysis)
- Attacker achieves initial code execution in a sandboxed renderer process.
- The compromised renderer binds the WebMIDI Mojo interface and calls
StartSession. - The browser process instantiates
MidiManagerWin, places the client inpending_clients_, and spawns the initialization task on a background thread. - Before initialization completes (which can be delayed due to slow Windows MIDI APIs like
midiInGetDevCaps), the renderer deliberately closes the Mojo pipe. - The browser process handles the disconnect, checks
HasOpenSession(), receivesfalse, and destructsMidiManagerWin. - The
~MidiManagerWindestructor races with the still-running background initialization, causing the memory corruption primitives detailed above.
Suggested Fix
- Fix Lifecycle Check: Update
MidiManager::HasOpenSession()inmedia/midi/midi_manager.ccto accurately reflect pending initialization states. It should check bothclients_andpending_clients_:bool MidiManager::HasOpenSession() { base::AutoLock auto_lock(lock_); return clients_.size() != 0u || pending_clients_.size() != 0u; } - Synchronize Destructor: In
media/midi/midi_manager_win.cc, ensure thatGetTaskLock()is acquired before iterating over the port lists in~MidiManagerWin(), preventing the data race entirely if premature destruction somehow occurs.
Evaluated with Chrome root at commit: f200f57a19490707ff8bc7aa5de3cbc443a3afad
Results so far have been promising, but there can be wrong deductions. If this proves to be a false positive, please close as WAI; data from false positives will be used to improve accuracy over time. And please feel free to reach out to me directly if you have concerns or feedback on the project.