Overview

High
Severity
CVSS
No
Exploited ITW
Fixed
Fix Status
ImpactUse after free in WebMIDI
DescriptionUse after free in WebMIDI
ComponentWebMIDI
Bug ClassUAF
Tracker514742327
Fix commit2f0dc63af5b1 (chromium/src) +90/-19
CISA KEVNot listed
CreditedGoogle
Disclosed2026-05-27

Changed Functions

FunctionChangeNotes
for
media/midi/midi_manager_android.cc
modified
MidiManagerAndroid
media/midi/midi_manager_android.h
modified
MIDI_EXPORT
media/midi/midi_manager_android.h
modified

Files Changed

  • media/midi/OWNERS
  • media/midi/midi_manager_android.cc
  • media/midi/midi_manager_android.h
  • media/midi/midi_manager_unittest.cc
From 2f0dc63af5b10d63243a451b8043999f2b680e8c Mon Sep 17 00:00:00 2001
From: Hongchan Choi <hongchan@google.com>
Date: Thu, 21 May 2026 12:19:22 -0700
Subject: [PATCH] [media/midi] Synchronize access to MidiManagerAndroid ports and devices

Synchronize access to all input/output port vectors, port maps,
and device vectors in MidiManagerAndroid using the existing lock_.

Previously, these collections were mutated on the UI thread (inside
AddDevice and OnDetached) but accessed on the IO thread (inside
DispatchSendMidiData) and JNI threads (inside OnReceivedData) without
synchronization. This could lead to concurrent access issues.

We resolve this by:
- Guarding all shared collection accesses in MidiManagerAndroid
  under the internal lock_.
- Using extremely short critical sections (bounds check, vector read,
  or map lookup only) on the real-time paths (DispatchSendMidiData
  and OnReceivedData) to ensure zero real-time overhead.
- Keeping expensive operations, JNI calls, and client notifications
  completely outside of locked sections, which also eliminates nested
  lock acquisitions and prevents deadlocks.
- Adding a multi-threaded concurrency unit test in
  midi_manager_unittest.cc to verify thread safety.

Bug: 514742327
Test: midi_unittests
Change-Id: Ic4ee8d17dee46819888800dbea11a5a9144e1d0d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7861363
Commit-Queue: Hongchan Choi <hongchan@chromium.org>
Reviewed-by: Michael Wilson <mjwilson@chromium.org>
Reviewed-by: Takashi Toyoshima <toyoshim@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1634454}
---

diff --git a/media/midi/OWNERS b/media/midi/OWNERS
index 19e2d23..d7c888b8 100644
--- a/media/midi/OWNERS
+++ b/media/midi/OWNERS
@@ -1,6 +1,8 @@
 # Following reviewers should be able to review all code under //media/midi.
 # But it would be a good idea to involve {yukawa,agoode}@chromium.org if
 # they are the original author of files you are touching.
+hongchan@chromium.org
+mjwilson@chromium.org
 toyoshim@chromium.org
 
 per-file *.mojom=set noparent
diff --git a/media/midi/midi_manager_android.cc b/media/midi/midi_manager_android.cc
index bd68160..36ca6e1 100644
--- a/media/midi/midi_manager_android.cc
+++ b/media/midi/midi_manager_android.cc
@@ -76,15 +76,21 @@
                                               uint32_t port_index,
                                               const std::vector<uint8_t>& data,
                                               base::TimeTicks timestamp) {
-  if (port_index >= all_output_ports_.size()) {
-    // |port_index| is provided by a renderer so we can't believe that it is
-    // in the valid range.
-    return;
+  MidiOutputPortAndroid* port = nullptr;
+  {
+    base::AutoLock auto_lock(lock_);
+    if (port_index >= all_output_ports_.size()) {
+      // |port_index| is provided by a renderer so we can't believe that it is
+      // in the valid range.
+      return;
+    }
+    port = all_output_ports_[port_index];
   }
+
   if (GetOutputPortState(port_index) == PortState::CONNECTED) {
     // We treat send call as implicit open.
     // TODO(yhirano): Implement explicit open operation from the renderer.
-    if (all_output_ports_[port_index]->Open()) {
+    if (port->Open()) {
       SetOutputPortState(port_index, PortState::OPENED);
     } else {
       // We cannot open the port. It's useless to send data to such a port.
@@ -99,7 +105,7 @@
   service()->task_service()->PostBoundDelayedTask(
       TaskService::kDefaultRunnerId,
       base::BindOnce(&MidiOutputPortAndroid::Send,
-                     base::Unretained(all_output_ports_[port_index]), data),
+                     base::Unretained(port), data),
       delay);
   service()->task_service()->PostBoundDelayedTask(
       TaskService::kDefaultRunnerId,
@@ -111,9 +117,14 @@
 void MidiManagerAndroid::OnReceivedData(MidiInputPortAndroid* port,
                                         base::span<const uint8_t> data,
                                         base::TimeTicks timestamp) {
-  const auto i = input_port_to_index_.find(port);
-  DCHECK(input_port_to_index_.end() != i);
-  ReceiveMidiData(i->second, data, timestamp);
+  size_t index = 0;
+  {
+    base::AutoLock auto_lock(lock_);
+    const auto i = input_port_to_index_.find(port);
+    DCHECK(input_port_to_index_.end() != i);
+    index = i->second;
+  }
+  ReceiveMidiData(index, data, timestamp);
 }
 
 void MidiManagerAndroid::OnInitialized(JNIEnv* env,
@@ -141,6 +152,7 @@
 
 void MidiManagerAndroid::OnDetached(JNIEnv* env,
                                     const JavaRef<jobject>& raw_device) {
+  base::AutoLock auto_lock(lock_);
   for (auto& device : devices_) {
     if (device->HasRawDevice(env, raw_device)) {
       for (auto& port : device->input_ports()) {
@@ -160,6 +172,7 @@
 }
 
 void MidiManagerAndroid::AddDevice(std::unique_ptr<MidiDeviceAndroid> device) {
+  base::AutoLock auto_lock(lock_);
   for (auto& port : device->input_ports()) {
     // We implicitly open input ports here, because there are no signal
     // from the renderer when to open.
diff --git a/media/midi/midi_manager_android.h b/media/midi/midi_manager_android.h
index 4a46cfa..be774fd 100644
--- a/media/midi/midi_manager_android.h
+++ b/media/midi/midi_manager_android.h
@@ -16,6 +16,7 @@
 #include "base/containers/span.h"
 #include "base/memory/raw_ptr.h"
 #include "base/synchronization/lock.h"
+#include "base/thread_annotations.h"
 #include "base/time/time.h"
 #include "media/midi/midi_export.h"
 #include "media/midi/midi_input_port_android.h"
@@ -34,8 +35,9 @@
 
 // MidiManagerAndroid is a MidiManager subclass for Android M or newer. For
 // older android OSes, we use MidiManagerUsb.
-class MidiManagerAndroid final : public MidiManager,
-                                 public MidiInputPortAndroid::Delegate {
+class MIDI_EXPORT MidiManagerAndroid final
+    : public MidiManager,
+      public MidiInputPortAndroid::Delegate {
  public:
   explicit MidiManagerAndroid(MidiService* service);
   ~MidiManagerAndroid() override;
@@ -66,30 +68,30 @@
   void AddOutputPortAndroid(MidiOutputPortAndroid* port,
                             MidiDeviceAndroid* device);
 
-  // TODO(toyoshim): Remove |lock_| once dynamic instantiation mode is enabled
-  // by default. This protects objects allocated on the I/O thread from doubly
-  // released on the main thread.
+  // Protects objects mutated on the UI thread but accessed on the IO thread.
   base::Lock lock_;
 
   // All ports held in |devices_|. Each device has ownership of ports, but we
   // can store pointers here because a device will keep its ports while it is
   // alive.
   std::vector<raw_ptr<MidiInputPortAndroid, VectorExperimental>>
-      all_input_ports_;
+      all_input_ports_ GUARDED_BY(lock_);
   // A dictionary from a port to its index.
   // input_port_to_index_[all_input_ports_[i]] == i for each valid |i|.
-  absl::flat_hash_map<MidiInputPortAndroid*, size_t> input_port_to_index_;
+  absl::flat_hash_map<MidiInputPortAndroid*, size_t> input_port_to_index_
+      GUARDED_BY(lock_);
 
   // Ditto for output ports.
   std::vector<raw_ptr<MidiOutputPortAndroid, VectorExperimental>>
-      all_output_ports_;
-  absl::flat_hash_map<MidiOutputPortAndroid*, size_t> output_port_to_index_;
+      all_output_ports_ GUARDED_BY(lock_);
+  absl::flat_hash_map<MidiOutputPortAndroid*, size_t> output_port_to_index_
+      GUARDED_BY(lock_);
 
   // `devices_` must be declared after the port index maps so that it is
   // destroyed first (in reverse declaration order). This ensures that MidiPorts
   // are closed and stop receiving callbacks before the maps are destroyed.
   // See https://crbug.com/490254128.
-  std::vector<std::unique_ptr<MidiDeviceAndroid>> devices_;
+  std::vector<std::unique_ptr<MidiDeviceAndroid>> devices_ GUARDED_BY(lock_);
 
   base::android::ScopedJavaGlobalRef<jobject> raw_manager_;
 };
diff --git a/media/midi/midi_manager_unittest.cc b/media/midi/midi_manager_unittest.cc
index 9eb48aa..2d4aff3 100644
--- a/media/midi/midi_manager_unittest.cc
+++ b/media/midi/midi_manager_unittest.cc
@@ -28,6 +28,8 @@
 #endif  // BUILDFLAG(IS_WIN)
 
 #if BUILDFLAG(IS_ANDROID)
+#include "base/android/jni_android.h"
+#include "base/threading/simple_thread.h"
 #include "media/midi/midi_manager_android.h"
 #endif  // BUILDFLAG(IS_ANDROID)
 
@@ -444,6 +446,58 @@
   EndSession();
Loading diff…

Regression Test / PoC

shipped with the fix
diff --git a/media/midi/midi_manager_unittest.cc b/media/midi/midi_manager_unittest.cc
index 9eb48aa..2d4aff3 100644
--- a/media/midi/midi_manager_unittest.cc
+++ b/media/midi/midi_manager_unittest.cc
@@ -28,6 +28,8 @@
 #endif  // BUILDFLAG(IS_WIN)
 
 #if BUILDFLAG(IS_ANDROID)
+#include "base/android/jni_android.h"
+#include "base/threading/simple_thread.h"
 #include "media/midi/midi_manager_android.h"
 #endif  // BUILDFLAG(IS_ANDROID)
 
@@ -444,6 +446,58 @@
   EndSession();
 }
 
+#if BUILDFLAG(IS_ANDROID)
+class ConcurrencyTestThread : public base::SimpleThread {
+ public:
+  ConcurrencyTestThread(const std::string& name, base::OnceClosure closure)
+      : base::SimpleThread(name), closure_(std::move(closure)) {}
+  void Run() override { std::move(closure_).Run(); }
+ private:
+  base::OnceClosure closure_;
+};
+
+TEST_F(MidiManagerTest, MidiManagerAndroidConcurrency) {
+  auto manager = std::make_unique<MidiManagerAndroid>(service());
+  std::atomic<bool> stop{false};
+
+  // SendThread: Repeatedly sends MIDI data. Since the list of output ports is
+  // empty, this quickly bounds-checks and exits under the lock, simulating
+  // high-frequency concurrent messaging activity.
+  base::OnceClosure send_task = base::BindOnce(
+      [](MidiManagerAndroid* manager, std::atomic<bool>* stop) {
+        std::vector<uint8_t> data;
+        while (!stop->load()) {
+          manager->DispatchSendMidiData(nullptr, 0, data, base::TimeTicks());
+        }
+      },
+      manager.get(), &stop);
+
+  // DetachThread: Repeatedly simulates detaching a device. Since the list
+  // of devices is empty, this quickly exits under the lock, simulating
+  // concurrent dynamic device-attach/detach JNI events.
+  base::OnceClosure detach_task = base::BindOnce(
+      [](MidiManagerAndroid* manager, std::atomic<bool>* stop) {
+        JNIEnv* env = base::android::AttachCurrentThread();
+        while (!stop->load()) {
+          manager->OnDetached(env, nullptr);
+        }
+      },
+      manager.get(), &stop);
+
+  ConcurrencyTestThread thread1("SendThread", std::move(send_task));
+  ConcurrencyTestThread thread2("DetachThread", std::move(detach_task));
+
+  thread1.Start();
+  thread2.Start();
+
+  base::PlatformThread::Sleep(base::Milliseconds(100));
+  stop.store(true);
+
+  thread1.Join();
+  thread2.Join();
+}
+#endif  // BUILDFLAG(IS_ANDROID)
+
 }  // namespace
 
 }  // namespace midi
Loading diff…

Original Bug Report

reported by vm...@google.com

Potential Race Condition and UAF in MidiManagerAndroid leading to JNI Type Confusion

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. Please see https://chromium.googlesource.com/chromium/src/+/main/docs/security/ai-generated-security-bugs-faq.md for more information.

Overview: A race condition in MidiManagerAndroid allows for a potential Use-After-Free (UAF) on a vector’s backing store due to unsynchronized access between the Android UI and Chrome IO threads. This vulnerability could be leveraged to achieve JNI type confusion within the browser process. The issue stems from mutating MIDI port vectors during device attachment while concurrently reading them to dispatch MIDI data.

Affected files:

  • media/midi/midi_manager_android.cc
  • media/midi/midi_manager_android.h

Estimated timestamp from git blame: Unknown (Google3 checkout)

Summary

A potential race condition exists in MidiManagerAndroid because access to the all_output_ports_ vector is not synchronized between the Android UI thread and the Chrome IO thread. This can result in a Use-After-Free (UAF) during vector reallocation, which may lead to JNI type confusion in the unsandboxed browser process.

Technical Details

MidiManagerAndroid manages MIDI output ports using a std::vector<raw_ptr<MidiOutputPortAndroid>> all_output_ports_. While the class defines a base::Lock lock_, this lock is not acquired during many critical operations on the port vectors.

1. Mutation on the UI Thread: When a MIDI device is attached, the Android framework triggers JNI callbacks on the UI thread. This eventually calls MidiManagerAndroid::OnAttached (in media/midi/midi_manager_android.cc), which invokes AddDevice. Inside AddDevice, the code performs all_output_ports_.push_back(port.get()) without any locking.

2. Concurrent Read on the IO Thread: Renderer-initiated MIDI data transfers (via Mojo SendData) are handled by MidiHost on the IO thread. This calls MidiManagerAndroid::DispatchSendMidiData, which reads from the vector using all_output_ports_[port_index] to retrieve a port pointer, also without holding a lock.

3. The Race and UAF: If the push_back on the UI thread triggers a vector reallocation, the vector’s internal backing storage is deallocated and replaced. If the IO thread is concurrently reading from the vector, it may access the old storage after it has been freed.

Notably, while the elements are raw_ptr, MiraclePtr (BackupRefPtr) protects the objects the pointers point to, but not the internal backing array of the std::vector itself. Thus, the UAF occurs on the buffer containing the pointers.

Potential Exploitation Primitive

An attacker could potentially escalate this UAF to JNI type confusion:

  1. Pointer Capture: The IO thread reads a garbage value from the freed vector storage and binds it as a base::Unretained pointer into a delayed task.
  2. Heap Reclamation: By providing a timestamp in the Mojo call, the renderer can delay the task’s execution, creating a window to spray the browser process heap and reclaim the freed storage with controlled data.
  3. JNI Type Confusion: When the task executes, it calls MidiOutputPortAndroid::Send on the controlled pointer. This method reads a jobject handle from offset 0 (the location of ScopedJavaGlobalRef<jobject> raw_port_) and passes it to Java_MidiOutputPortAndroid_send. This allows the attacker to invoke JNI methods using an arbitrary object handle.

Suggested Potential Steps to Trigger

Note: These steps are theoretical as our tooling cannot currently execute code.

  1. A compromised renderer process obtains MIDI permissions.
  2. The renderer begins a high-frequency stream of MIDI data to an existing port.
  3. Simultaneously, an external event (such as a malicious App cycling virtual MIDI devices on the same Android device) triggers frequent MIDI device-attach notifications.
  4. The resulting race triggers a vector reallocation in AddDevice while DispatchSendMidiData is active.
  5. The attacker attempts to groom the browser heap to control the contents of the freed vector buffer during the reallocation window.

Suggested Fix

Ensure that all accesses (reads and writes) to all_input_ports_, all_output_ports_, and their associated index maps in MidiManagerAndroid are protected by a base::Lock. The existing lock_ in the class should be utilized for this purpose.

Evaluated with Chrome root at commit: 29093e11cf509e3593f6229e4b1b075cca356049


Results so far have been promising, but there can be wrong deductions. Feel free to adjust as follows:

  • If you are familiar with the severity guidelines, you may adjust the severity.
  • If this is a false positive, and there’s no work to be done, please close as WAI.
  • If there is work to do here but not a vulnerability, please change the issue type to Task/Bug/FR.

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.

View on issue tracker