Medium chrome Logic Error 📄 Reporter bug report 🔧 Commit mapped

Overview

Medium
Severity
CVSS
No
Exploited ITW
Fixed
Fix Status
ImpactImproper enforcement of behavioral workflow in Media
DescriptionImproper enforcement of behavioral workflow in Media
ComponentMedia
Bug ClassLogic Error
Tracker519984038
Fix commit08a430db2ecf (chromium/src) +336/-391
CISA KEVNot listed
CreditedGoogle
Disclosed2026-08-25

Changed Functions

FunctionChangeNotes
TEST_F
media/mojo/clients/mojo_decryptor_unittest.cc
modified

Files Changed

  • media/mojo/clients/mojo_decryptor_unittest.cc
  • media/mojo/services/mojo_decryptor_service.cc
From 08a430db2ecfa551523c34f3ba0cf4fc5435958b Mon Sep 17 00:00:00 2001
From: Vikram Pasupathy <vpasupathy@chromium.org>
Date: Thu, 09 Jul 2026 12:07:49 -0700
Subject: [PATCH] media: Fix missing HasPendingReads guard in MojoDecryptorService

A compromised renderer can stall the data pipe during a DecryptAndDecode
read and re-initialize the decoder, causing the pending decode to
execute against the new decoder context.

A previous attempt to fix this added a HasPendingReads() check that
triggered a mojo::ReportBadMessage(). I had to revert that (in
79c6995f74579e21a94b2801d82260838b33bb51) because it was causing crashes
on ChromeOS. When ChromeOS stops a failing hardware decoder to try a
software CDM, it fires InitializeVideoDecoder while data from the
stopped attempt is still draining. This hits the check and crashes the
renderer.

To fix this safely, we now use a WeakPtrFactory invalidation pattern to
drop pending reads during recovery without crashing.

Instead of a single DecoderHandler multiplexing both audio and video,
the service now uses dedicated AudioStream and VideoStream inner
classes. Each stream class explicitly manages its own isolated
WeakPtrFactory domains (one for Decrypt() and one for
DecryptAndDecode()), providing a clean 1:1 mapping with the media
pipeline and ensuring that resetting one stream never drops callbacks
for the other.

Bug: 519984038
Change-Id: Ia3c28439499061a943f91c5325ffb2c6e4d35896
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7927806
Reviewed-by: Nathan Hebert <nhebert@chromium.org>
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Commit-Queue: Vikram Pasupathy <vpasupathy@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1659743}
---

diff --git a/media/mojo/clients/mojo_decryptor_unittest.cc b/media/mojo/clients/mojo_decryptor_unittest.cc
index 75727619..1643e0f 100644
--- a/media/mojo/clients/mojo_decryptor_unittest.cc
+++ b/media/mojo/clients/mojo_decryptor_unittest.cc
@@ -131,12 +131,12 @@
   // The MojoDecryptor that we are testing.
   std::unique_ptr<MojoDecryptor> mojo_decryptor_;
 
+  // The actual Decryptor object used by |mojo_decryptor_service_|.
+  std::unique_ptr<StrictMock<MockDecryptor>> decryptor_;
+
   // The matching MojoDecryptorService for |mojo_decryptor_|.
   std::unique_ptr<MojoDecryptorService> mojo_decryptor_service_;
   std::unique_ptr<mojo::Receiver<mojom::Decryptor>> receiver_;
-
-  // The actual Decryptor object used by |mojo_decryptor_service_|.
-  std::unique_ptr<StrictMock<MockDecryptor>> decryptor_;
 };
 
 // DecryptAndDecodeAudio() and ResetDecoder(kAudio) immediately.
@@ -414,96 +414,42 @@
 }
 
 TEST_F(MojoDecryptorTest, Deinitialize_DuringDecryptAndDecode) {
-  decryptor_ = std::make_unique<StrictMock<MockDecryptor>>();
-  mojo_decryptor_service_ =
-      std::make_unique<MojoDecryptorService>(decryptor_.get(), nullptr);
+  SetWriterCapacity(20);
+  Initialize();
 
-  mojo::Remote<mojom::Decryptor> remote_decryptor;
-  receiver_ = std::make_unique<mojo::Receiver<mojom::Decryptor>>(
-      mojo_decryptor_service_.get(),
-      remote_decryptor.BindNewPipeAndPassReceiver());
+  base::RunLoop run_loop;
+  EXPECT_CALL(*this, AudioDecoded(Decryptor::kError, _))
+      .WillOnce(testing::InvokeWithoutArgs(&run_loop, &base::RunLoop::Quit));
 
-  // Create the four DataPipes.
-  mojo::ScopedDataPipeConsumerHandle audio_consumer;
-  mojo::ScopedDataPipeProducerHandle audio_producer;
-  ASSERT_EQ(MOJO_RESULT_OK,
-            mojo::CreateDataPipe(nullptr, audio_producer, audio_consumer));
+  auto buffer = DecoderBuffer::CopyFrom(std::vector<uint8_t>(100, 0));
+  mojo_decryptor_->DecryptAndDecodeAudio(
+      std::move(buffer), base::BindRepeating(&MojoDecryptorTest::AudioDecoded,
+                                             base::Unretained(this)));
 
-  mojo::ScopedDataPipeConsumerHandle video_consumer;
-  mojo::ScopedDataPipeProducerHandle video_producer;
-  ASSERT_EQ(MOJO_RESULT_OK,
-            mojo::CreateDataPipe(nullptr, video_producer, video_consumer));
-
-  mojo::ScopedDataPipeConsumerHandle decrypt_consumer;
-  mojo::ScopedDataPipeProducerHandle decrypt_producer;
-  ASSERT_EQ(MOJO_RESULT_OK,
-            mojo::CreateDataPipe(nullptr, decrypt_producer, decrypt_consumer));
-
-  mojo::ScopedDataPipeProducerHandle decrypted_producer;
-  mojo::ScopedDataPipeConsumerHandle decrypted_consumer;
-  ASSERT_EQ(MOJO_RESULT_OK, mojo::CreateDataPipe(nullptr, decrypted_producer,
-                                                 decrypted_consumer));
-
-  remote_decryptor->Initialize(
-      std::move(audio_consumer), std::move(video_consumer),
-      std::move(decrypt_consumer), std::move(decrypted_producer));
-
-  bool deinitialized = false;
-
-  EXPECT_CALL(*decryptor_, InitializeAudioDecoder(_, _))
-      .Times(testing::AtMost(1))
-      .WillOnce([](const AudioDecoderConfig& config,
-                   Decryptor::DecoderInitCB cb) { std::move(cb).Run(true); });
-
-  EXPECT_CALL(*decryptor_, DeinitializeDecoder(Decryptor::kAudio))
-      .Times(testing::AtMost(1))
-      .WillOnce(testing::Assign(&deinitialized, true));
-
-  EXPECT_CALL(*decryptor_, DecryptAndDecodeAudio(_, _))
-      .WillRepeatedly([&deinitialized](scoped_refptr<DecoderBuffer> buffer,
-                                       Decryptor::AudioDecodeCB cb) {
-        ASSERT_FALSE(deinitialized) << "DecryptAndDecodeAudio called after "
-                                       "DeinitializeDecoder!";
-        std::move(cb).Run(Decryptor::kSuccess, Decryptor::AudioFrames());
-      });
-
-  auto data_buffer = mojom::DataDecoderBuffer::New();
-  data_buffer->timestamp = base::TimeDelta();
-  data_buffer->duration = base::Seconds(1);
-  data_buffer->is_key_frame = false;
-  data_buffer->data_size = 256;
-
-  auto mojo_buffer = mojom::DecoderBuffer::NewData(std::move(data_buffer));
-
-  bool decode_called = false;
-  remote_decryptor->DecryptAndDecodeAudio(
-      std::move(mojo_buffer),
-      base::BindOnce(
-          [](bool* called, Decryptor::Status status,
-             std::vector<mojom::AudioBufferPtr> buffers) { *called = true; },
-          &decode_called));
-
-  mojo::test::BadMessageObserver bad_message_observer;
-  remote_decryptor->DeinitializeDecoder(Decryptor::kAudio);
-
-  remote_decryptor->InitializeAudioDecoder(
-      AudioDecoderConfig(AudioCodec::kAAC, SampleFormat::kSampleFormatS16,
-                         ChannelLayoutConfig::Stereo(), 44100,
-                         std::vector<uint8_t>(),
-                         EncryptionScheme::kUnencrypted),
-      base::BindOnce(
-          [](mojo::ScopedDataPipeProducerHandle producer, bool success) {
-            LOG(INFO) << "InitializeAudioDecoder callback called. Success="
-                      << success;
-            std::vector<uint8_t> data(256, 0);
-            size_t bytes_written = 0;
-            std::ignore = producer->WriteData(data, MOJO_WRITE_DATA_FLAG_NONE,
-                                              bytes_written);
-          },
-          std::move(audio_producer)));
-
-  std::string bad_message = bad_message_observer.WaitForBadMessage();
-  EXPECT_EQ(bad_message,
-            "DeinitializeDecoder with pending DecryptAndDecode reads");
+  EXPECT_CALL(*decryptor_, DeinitializeDecoder(Decryptor::kAudio));
+  mojo_decryptor_->DeinitializeDecoder(Decryptor::kAudio);
+  run_loop.Run();
 }
+
+TEST_F(MojoDecryptorTest, InitializeVideoDecoder_DuringDecryptAndDecode) {
+  SetWriterCapacity(20);
+  Initialize();
+
+  base::RunLoop run_loop;
+  EXPECT_CALL(*this, VideoDecoded(Decryptor::kError, IsNull()))
+      .WillOnce(testing::InvokeWithoutArgs(&run_loop, &base::RunLoop::Quit));
+
+  auto buffer = DecoderBuffer::CopyFrom(std::vector<uint8_t>(100, 0));
+  mojo_decryptor_->DecryptAndDecodeVideo(
+      std::move(buffer), base::BindRepeating(&MojoDecryptorTest::VideoDecoded,
+                                             base::Unretained(this)));
+
+  EXPECT_CALL(*decryptor_, InitializeVideoDecoder(_, _))
+      .WillOnce([](const VideoDecoderConfig& config,
+                   Decryptor::DecoderInitCB cb) { std::move(cb).Run(true); });
+  mojo_decryptor_->InitializeVideoDecoder(TestVideoConfig::Normal(),
+                                          base::DoNothing());
+  run_loop.Run();
+}
+
 }  // namespace media
diff --git a/media/mojo/services/mojo_decryptor_service.cc b/media/mojo/services/mojo_decryptor_service.cc
index 9e1f06fe..d43f18a23 100644
--- a/media/mojo/services/mojo_decryptor_service.cc
+++ b/media/mojo/services/mojo_decryptor_service.cc
@@ -6,8 +6,11 @@
 
 #include <memory>
 #include <utility>
+#include <vector>
 
 #include "base/functional/bind.h"
+#include "base/functional/callback_helpers.h"
Loading diff…

Regression Test / PoC

shipped with the fix
diff --git a/media/mojo/clients/mojo_decryptor_unittest.cc b/media/mojo/clients/mojo_decryptor_unittest.cc
index 75727619..1643e0f 100644
--- a/media/mojo/clients/mojo_decryptor_unittest.cc
+++ b/media/mojo/clients/mojo_decryptor_unittest.cc
@@ -131,12 +131,12 @@
   // The MojoDecryptor that we are testing.
   std::unique_ptr<MojoDecryptor> mojo_decryptor_;
 
+  // The actual Decryptor object used by |mojo_decryptor_service_|.
+  std::unique_ptr<StrictMock<MockDecryptor>> decryptor_;
+
   // The matching MojoDecryptorService for |mojo_decryptor_|.
   std::unique_ptr<MojoDecryptorService> mojo_decryptor_service_;
   std::unique_ptr<mojo::Receiver<mojom::Decryptor>> receiver_;
-
-  // The actual Decryptor object used by |mojo_decryptor_service_|.
-  std::unique_ptr<StrictMock<MockDecryptor>> decryptor_;
 };
 
 // DecryptAndDecodeAudio() and ResetDecoder(kAudio) immediately.
@@ -414,96 +414,42 @@
 }
 
 TEST_F(MojoDecryptorTest, Deinitialize_DuringDecryptAndDecode) {
-  decryptor_ = std::make_unique<StrictMock<MockDecryptor>>();
-  mojo_decryptor_service_ =
-      std::make_unique<MojoDecryptorService>(decryptor_.get(), nullptr);
+  SetWriterCapacity(20);
+  Initialize();
 
-  mojo::Remote<mojom::Decryptor> remote_decryptor;
-  receiver_ = std::make_unique<mojo::Receiver<mojom::Decryptor>>(
-      mojo_decryptor_service_.get(),
-      remote_decryptor.BindNewPipeAndPassReceiver());
+  base::RunLoop run_loop;
+  EXPECT_CALL(*this, AudioDecoded(Decryptor::kError, _))
+      .WillOnce(testing::InvokeWithoutArgs(&run_loop, &base::RunLoop::Quit));
 
-  // Create the four DataPipes.
-  mojo::ScopedDataPipeConsumerHandle audio_consumer;
-  mojo::ScopedDataPipeProducerHandle audio_producer;
-  ASSERT_EQ(MOJO_RESULT_OK,
-            mojo::CreateDataPipe(nullptr, audio_producer, audio_consumer));
+  auto buffer = DecoderBuffer::CopyFrom(std::vector<uint8_t>(100, 0));
+  mojo_decryptor_->DecryptAndDecodeAudio(
+      std::move(buffer), base::BindRepeating(&MojoDecryptorTest::AudioDecoded,
+                                             base::Unretained(this)));
 
-  mojo::ScopedDataPipeConsumerHandle video_consumer;
-  mojo::ScopedDataPipeProducerHandle video_producer;
-  ASSERT_EQ(MOJO_RESULT_OK,
-            mojo::CreateDataPipe(nullptr, video_producer, video_consumer));
-
-  mojo::ScopedDataPipeConsumerHandle decrypt_consumer;
-  mojo::ScopedDataPipeProducerHandle decrypt_producer;
-  ASSERT_EQ(MOJO_RESULT_OK,
-            mojo::CreateDataPipe(nullptr, decrypt_producer, decrypt_consumer));
-
-  mojo::ScopedDataPipeProducerHandle decrypted_producer;
-  mojo::ScopedDataPipeConsumerHandle decrypted_consumer;
-  ASSERT_EQ(MOJO_RESULT_OK, mojo::CreateDataPipe(nullptr, decrypted_producer,
-                                                 decrypted_consumer));
-
-  remote_decryptor->Initialize(
-      std::move(audio_consumer), std::move(video_consumer),
-      std::move(decrypt_consumer), std::move(decrypted_producer));
-
-  bool deinitialized = false;
-
-  EXPECT_CALL(*decryptor_, InitializeAudioDecoder(_, _))
-      .Times(testing::AtMost(1))
-      .WillOnce([](const AudioDecoderConfig& config,
-                   Decryptor::DecoderInitCB cb) { std::move(cb).Run(true); });
-
-  EXPECT_CALL(*decryptor_, DeinitializeDecoder(Decryptor::kAudio))
-      .Times(testing::AtMost(1))
-      .WillOnce(testing::Assign(&deinitialized, true));
-
-  EXPECT_CALL(*decryptor_, DecryptAndDecodeAudio(_, _))
-      .WillRepeatedly([&deinitialized](scoped_refptr<DecoderBuffer> buffer,
-                                       Decryptor::AudioDecodeCB cb) {
-        ASSERT_FALSE(deinitialized) << "DecryptAndDecodeAudio called after "
-                                       "DeinitializeDecoder!";
-        std::move(cb).Run(Decryptor::kSuccess, Decryptor::AudioFrames());
-      });
-
-  auto data_buffer = mojom::DataDecoderBuffer::New();
-  data_buffer->timestamp = base::TimeDelta();
-  data_buffer->duration = base::Seconds(1);
-  data_buffer->is_key_frame = false;
-  data_buffer->data_size = 256;
-
-  auto mojo_buffer = mojom::DecoderBuffer::NewData(std::move(data_buffer));
-
-  bool decode_called = false;
-  remote_decryptor->DecryptAndDecodeAudio(
-      std::move(mojo_buffer),
-      base::BindOnce(
-          [](bool* called, Decryptor::Status status,
-             std::vector<mojom::AudioBufferPtr> buffers) { *called = true; },
-          &decode_called));
-
-  mojo::test::BadMessageObserver bad_message_observer;
-  remote_decryptor->DeinitializeDecoder(Decryptor::kAudio);
-
-  remote_decryptor->InitializeAudioDecoder(
-      AudioDecoderConfig(AudioCodec::kAAC, SampleFormat::kSampleFormatS16,
-                         ChannelLayoutConfig::Stereo(), 44100,
-                         std::vector<uint8_t>(),
-                         EncryptionScheme::kUnencrypted),
-      base::BindOnce(
-          [](mojo::ScopedDataPipeProducerHandle producer, bool success) {
-            LOG(INFO) << "InitializeAudioDecoder callback called. Success="
-                      << success;
-            std::vector<uint8_t> data(256, 0);
-            size_t bytes_written = 0;
-            std::ignore = producer->WriteData(data, MOJO_WRITE_DATA_FLAG_NONE,
-                                              bytes_written);
-          },
-          std::move(audio_producer)));
-
-  std::string bad_message = bad_message_observer.WaitForBadMessage();
-  EXPECT_EQ(bad_message,
-            "DeinitializeDecoder with pending DecryptAndDecode reads");
+  EXPECT_CALL(*decryptor_, DeinitializeDecoder(Decryptor::kAudio));
+  mojo_decryptor_->DeinitializeDecoder(Decryptor::kAudio);
+  run_loop.Run();
 }
+
+TEST_F(MojoDecryptorTest, InitializeVideoDecoder_DuringDecryptAndDecode) {
+  SetWriterCapacity(20);
+  Initialize();
+
+  base::RunLoop run_loop;
+  EXPECT_CALL(*this, VideoDecoded(Decryptor::kError, IsNull()))
+      .WillOnce(testing::InvokeWithoutArgs(&run_loop, &base::RunLoop::Quit));
+
+  auto buffer = DecoderBuffer::CopyFrom(std::vector<uint8_t>(100, 0));
+  mojo_decryptor_->DecryptAndDecodeVideo(
+      std::move(buffer), base::BindRepeating(&MojoDecryptorTest::VideoDecoded,
+                                             base::Unretained(this)));
+
+  EXPECT_CALL(*decryptor_, InitializeVideoDecoder(_, _))
+      .WillOnce([](const VideoDecoderConfig& config,
+                   Decryptor::DecoderInitCB cb) { std::move(cb).Run(true); });
+  mojo_decryptor_->InitializeVideoDecoder(TestVideoConfig::Normal(),
+                                          base::DoNothing());
+  run_loop.Run();
+}
+
 }  // namespace media
Loading diff…

Original Bug Report

reported by vm...@google.com

Missing HasPendingReads() Guard during MojoDecryptorService Decoder Re-initialization

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 potential security vulnerability in MojoDecryptorService allows a compromised renderer to bypass pending read state checks when re-initializing an audio or video decoder. By stalling a Mojo DataPipe, an attacker could queue a decode request, re-initialize the decoder with a different configuration, and then resume the pipe to execute the decode operation against mismatched decoder state. This state violation could potentially trigger memory corruption or a Use-After-Free (UAF) in the sandboxed library CDM process.

Affected files:

  • media/mojo/services/mojo_decryptor_service.cc
  • media/cdm/cdm_adapter.cc

Estimated timestamp from git blame: 2017-12-14

Description

A state-mutation guard bypass potentially exists in MojoDecryptorService. Chromium’s implementation of MojoDecryptorService::DeinitializeDecoder contains a safety check (HasPendingReads()) to prevent the decoder from being de-initialized while a decrypt-and-decode read is still pending:

// media/mojo/services/mojo_decryptor_service.cc
void MojoDecryptorService::DeinitializeDecoder(StreamType stream_type) {
  ...
  if (reader->HasPendingReads()) {
    CHECK(mojo::IsInMessageDispatch());
    mojo::ReportBadMessage(
        "DeinitializeDecoder with pending DecryptAndDecode reads");
    return;
  }
  decryptor_->DeinitializeDecoder(stream_type);
}

However, the sibling decoder entry points—InitializeAudioDecoder (lines 129–136) and InitializeVideoDecoder (lines 138–152)—do not implement any corresponding HasPendingReads() checks on their respective buffer readers.

Per the Content Decryption Module (CDM) API design (media/cdm/api/content_decryption_module.h), a library CDM allows a client to re-initialize an already-initialized decoder to adapt to dynamic configuration changes. Under normal circumstances, well-behaved clients like DecryptingVideoDecoder ensure no decodes are pending before calling initialize. However, a compromised renderer communicating directly over the mojom::Decryptor Mojo interface can bypass this sequence, causing a queued decode request to fire into a re-configured library CDM (e.g., Widevine), resulting in potential memory corruption.

Potential Attack Scenario

Because our tooling currently lacks the capability to compile and execute a functional Proof of Concept, the following steps represent a potential attack path based on static code analysis:

  1. Setup: A compromised renderer acquires a direct pending_remote<Decryptor> to the MojoDecryptorService running in the sandboxed utility process (sandbox::mojom::Sandbox::kCdm).
  2. Establish Data Pipes: The renderer calls Decryptor::Initialize and retains the producer ends of the audio/video Mojo data pipes.
  3. Initial Configuration: The renderer calls InitializeVideoDecoder with a valid initial configuration (e.g., H.264) and waits for successful completion.
  4. Queue a Stalled Read: The renderer sends a DecryptAndDecodeVideo request with an encrypted H.264 buffer but purposely stalls the Mojo data pipe by withholding the buffer payload bytes. On the utility process side, MojoDecoderBufferReader queues this read callback (OnVideoRead) and arms its pipe watcher (media/mojo/common/mojo_decoder_buffer_converter.cc). At this point, video_buffer_reader_->HasPendingReads() returns true.
  5. Re-initialize Decoder: The renderer calls InitializeVideoDecoder with a different configuration (e.g., VP9). Because InitializeVideoDecoder lacks a HasPendingReads() check, the request proceeds, causing the underlying CdmAdapter to instruct the library CDM to re-initialize for VP9. The CDM deallocates previous H.264 decoder contexts and sets up a VP9 context.
  6. Unstall and Trigger: The renderer writes the withheld H.264 bytes into the video data pipe. The pipe watcher wakes up, completes the read, and dispatches the parked DecryptAndDecodeVideo call to the library CDM. The CDM is forced to process the H.264 bitstream using VP9 decoder contexts, leading to potential out-of-bounds memory writes or a Use-After-Free (UAF) in the library CDM’s heap.

Suggested Fix

Validate that there are no pending reads prior to decoder re-initialization. Add HasPendingReads() checks to both InitializeAudioDecoder and InitializeVideoDecoder in media/mojo/services/mojo_decryptor_service.cc, reporting a bad message if a pending read is detected:

void MojoDecryptorService::InitializeAudioDecoder(
    const AudioDecoderConfig& config,
    InitializeAudioDecoderCallback callback) {
  DVLOG(1) << __func__;
  if (audio_buffer_reader_ && audio_buffer_reader_->HasPendingReads()) {
    CHECK(mojo::IsInMessageDispatch());
    mojo::ReportBadMessage(
        "InitializeAudioDecoder with pending DecryptAndDecode reads");
    return;
  }
  ...
}

Apply a symmetric guard to InitializeVideoDecoder as well.

Evaluated with Chrome root at commit: 57b021e1fdae94a215627d29aeb1ccf2eb5b3e91


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