Chrome · Media › Media
CVE-2026-79083
Logic Error in Media (Media)
Overview
Medium
Severity
7.5
CVSS
No
Exploited ITW
Fixed
Fix Status
Background
- `MojoDecryptorService`
- the service-side implementation of
mojom::Decryptor, which runs in the media/CDM service process and exposes amedia::Decryptor(CDM) to the renderer forDecrypt,DecryptAndDecodeAudio/DecryptAndDecodeVideo, decoder initialisation, reset and deinitialisation. - `MojoDecoderBufferReader` and data pipes
- encrypted buffer metadata arrives over Mojo IPC while the payload bytes arrive separately over a data pipe, so a
ReadDecoderBuffercall completes asynchronously only once the renderer has written enough bytes, and a renderer can stall it deliberately. - `WeakPtrFactory` invalidation
- calling
InvalidateWeakPtrs()turns every callback bound to aWeakPtrfrom that factory into a no-op, which drops in-flight async work that belongs to an older state. - `mojo::WrapCallbackWithDefaultInvokeIfNotRun`
- wraps a Mojo reply callback so it still runs, with given default (error) arguments, if the callback is destroyed without being invoked, so dropped work answers the renderer with
kErrorand does not leave it hanging.
Root Cause Analysis
As the commit message describes, a compromised renderer could start a
DecryptAndDecode read, stall the data pipe so ReadDecoderBuffer stayed pending, and then call InitializeVideoDecoder/InitializeAudioDecoder (or DeinitializeDecoder) on the same service. In the old design, read-completion callbacks (for example MojoDecryptorService::OnReadDone) were bound to a single, never-invalidated weak_this_, so when the stalled bytes finally arrived the pending buffer went into DecryptAndDecode against the newly initialised decoder context, breaking the workflow invariant that work issued for one decoder configuration must never run against another. A previous fix that called mojo::ReportBadMessage when HasPendingReads() was true was reverted because legitimate ChromeOS hardware-to-software CDM fallback re-initialises while data is still draining. The new code splits the service into per-stream MojoDecryptorService::Stream<kAudio> and Stream<kVideo> objects, each with separate decrypt_weak_factory_ and decode_weak_factory_. Stream::InitializeDecoder and Stream::DeinitializeDecoder call decode_weak_factory_.InvalidateWeakPtrs() before touching the decryptor, and Stream::CancelDecrypt invalidates decrypt_weak_factory_, so stale OnDecodeReadDone/OnDecryptReadDone continuations are silently dropped. Their wrapped reply callbacks then fire with Status::kError, which the new InitializeVideoDecoder_DuringDecryptAndDecode and Deinitialize_DuringDecryptAndDecode tests check.Key insight
Asynchronous decode reads were not tied to the decoder generation that issued them, so a stalled read could finish after re-initialisation and run on the new decoder. The fix scopes those continuations to a per-stream
WeakPtrFactory that is invalidated on every (de)initialisation, dropping stale reads with an error reply and not crashing.Attack Path
- Compromised renderer
The attacker needs a compromised renderer that can send raw
mojom::Decryptormessages and control the producer ends of the audio/video data pipes. Ordinary web content cannot drive this ordering directly. - Issue a decode and stall
The renderer sends
DecryptAndDecodeAudio/DecryptAndDecodeVideowith buffer metadata but withholds the payload bytes, leavingReadDecoderBufferpending in the service. - Re-initialise the decoder
While the read is pending, it sends
InitializeVideoDecoder/InitializeAudioDecoder(orDeinitializeDecoder) with a new configuration, which the service passes to the CDM’smedia::Decryptor. - Release the stalled read
The renderer then writes the withheld bytes; before the fix the completion callback forwarded the old buffer to
DecryptAndDecode*on the new decoder context (as the commit message states). - Decoder state confusion
Decoding data meant for a different or deinitialised decoder could trigger memory-safety bugs in the CDM or decoder inside the media/CDM service process
(inferred); the patch shows only the workflow violation, not a specific memory-corruption sink.
Impact Assessment
The bug lets a compromised renderer break the decoder lifecycle of
MojoDecryptorService, so buffers decode against a decoder context they were not issued for, inside the media/CDM service process (out of process from the renderer). Its main value is as a sandbox-escape building block toward a more privileged media or CDM process, which would require a further memory-safety primitive in the decoder or CDM and is (inferred). The advisory describes it only as “Improper enforcement of behavioral workflow in Media”, with no in-the-wild status, and protected-media (EME) playback with a decryptor-based CDM must be in use.Changed Functions
| Function | Change | Notes |
|---|---|---|
MojoDecryptorServicemedia/mojo/services/mojo_decryptor_service.cc |
modified | |
decrypt_writer_media/mojo/services/mojo_decryptor_service.cc |
modified | |
ifmedia/mojo/services/mojo_decryptor_service.cc |
modified | |
formedia/mojo/services/mojo_decryptor_service.cc |
modified |
Files Changed
media/mojo/clients/mojo_decryptor_unittest.ccmedia/mojo/services/mojo_decryptor_service.hmedia/mojo/services/mojo_decryptor_service.cc
Audit Directions
- Async reads spanning a state transitionIn other Mojo media services that read from data pipes (
MojoAudioDecoderService,MojoVideoDecoderService,MojoRendererService,MojoCdmService), check whetherInitialize/Resetinvalidates continuations of reads that are still pending, or whether a renderer-stalled read can complete into a newer configuration. - Single long-lived `weak_this_`Look for services that bind every callback to one
WeakPtrcreated in the constructor; such pointers only guard destruction, not logical generations, and should be paired with per-operation factories or generation counters. - Bad-message checks that real clients tripWhere a
ReportBadMessageguard had to be reverted for legitimate clients (such as ChromeOS decoder fallback), confirm the replacement fails safe by dropping or erroring, and that no other path still relies on the reverted guard.
Patch
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/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"
+#include "base/memory/raw_ptr.h"
#include "base/numerics/safe_conversions.h"
#include "media/base/audio_decoder_config.h"
#include "media/base/cdm_context.h"
@@ -19,6 +22,7 @@
#include "media/mojo/common/mojo_decoder_buffer_converter.h"
#include "media/mojo/mojom/demuxer_stream.mojom.h"
#include "media/mojo/services/mojo_cdm_service_context.h"
+#include "mojo/public/cpp/bindings/callback_helpers.h"
#include "mojo/public/cpp/bindings/pending_remote.h"
#include "mojo/public/cpp/bindings/self_owned_receiver.h"
@@ -51,17 +55,256 @@
const char kInvalidStateMessage[] = "MojoDecryptorService - invalid state";
+template <media::Decryptor::StreamType StreamTypeParam>
+struct StreamTraits;
+
+template <>
+struct StreamTraits<media::Decryptor::kAudio> {
+ using ConfigType = AudioDecoderConfig;
+ using InitCallback = mojom::Decryptor::InitializeAudioDecoderCallback;
+ using DecodeCallback = mojom::Decryptor::DecryptAndDecodeAudioCallback;
+
+ static void RunDecodeCallbackWithError(DecodeCallback callback) {
+ std::move(callback).Run(media::Decryptor::Status::kError,
+ std::vector<mojom::AudioBufferPtr>());
+ }
+};
+
+template <>
+struct StreamTraits<media::Decryptor::kVideo> {
+ using ConfigType = VideoDecoderConfig;
+ using InitCallback = mojom::Decryptor::InitializeVideoDecoderCallback;
+ using DecodeCallback = mojom::Decryptor::DecryptAndDecodeVideoCallback;
+
+ static void RunDecodeCallbackWithError(DecodeCallback callback) {
+ std::move(callback).Run(media::Decryptor::Status::kError, nullptr,
+ mojo::NullRemote());
+ }
+};
+
} // namespace
+template <MojoDecryptorService::StreamType StreamTypeParam>
+class MojoDecryptorService::Stream {
+ public:
+ using Traits = StreamTraits<StreamTypeParam>;
+
+ Stream(media::Decryptor* decryptor,
+ MojoDecoderBufferReader* shared_decrypt_reader,
+ MojoDecoderBufferWriter* shared_decrypt_writer)
+ : decryptor_(decryptor),
+ decrypt_reader_(shared_decrypt_reader),
+ decrypt_writer_(shared_decrypt_writer) {}
+
+ ~Stream() = default;
+
+ void InitializeDecodePipe(mojo::ScopedDataPipeConsumerHandle pipe) {
+ decode_reader_ = std::make_unique<MojoDecoderBufferReader>(std::move(pipe));
+ }
+
+ void Decrypt(mojom::DecoderBufferPtr encrypted, DecryptCallback callback) {
+ decrypt_reader_->ReadDecoderBuffer(
+ std::move(encrypted),
+ base::BindOnce(&Stream::OnDecryptReadDone,
+ decrypt_weak_factory_.GetWeakPtr(),
+ mojo::WrapCallbackWithDefaultInvokeIfNotRun(
+ std::move(callback), Status::kError, nullptr),
+ mojo::GetBadMessageCallback()));
+ }
+
+ void CancelDecrypt() {
+ decrypt_weak_factory_.InvalidateWeakPtrs();
+ decryptor_->CancelDecrypt(StreamTypeParam);
+ }
+
+ void InitializeDecoder(const typename Traits::ConfigType& config,
+ typename Traits::InitCallback callback) {
+ decode_weak_factory_.InvalidateWeakPtrs();
+
+ if constexpr (StreamTypeParam == StreamType::kVideo) {
+ if (!config.IsValidConfig()) {
+ std::move(callback).Run(false);
+ mojo::ReportBadMessage("Invalid VideoDecoderConfig");
+ return;
+ }
+ }
+
+ auto bound_cb = base::BindOnce(&Stream::OnDecoderInitialized,
+ decode_weak_factory_.GetWeakPtr(),
+ mojo::WrapCallbackWithDefaultInvokeIfNotRun(
+ std::move(callback), false));
+
+ if constexpr (StreamTypeParam == StreamType::kAudio) {
+ decryptor_->InitializeAudioDecoder(config, std::move(bound_cb));
+ } else {
+ decryptor_->InitializeVideoDecoder(config, std::move(bound_cb));
+ }
+ }
+
+ void DecryptAndDecode(mojom::DecoderBufferPtr encrypted,
+ typename Traits::DecodeCallback callback) {
+ if (!decode_reader_) {
+ mojo::ReportBadMessage(kInvalidStateMessage);
+ return;
+ }
+
+ auto wrapped_callback = WrapDecodeCallback(std::move(callback));
+
+ decode_reader_->ReadDecoderBuffer(
+ std::move(encrypted), base::BindOnce(&Stream::OnDecodeReadDone,
+ decode_weak_factory_.GetWeakPtr(),
+ mojo::GetBadMessageCallback(),
+ std::move(wrapped_callback)));
+ }
+
+ void ResetDecoder() {
+ if (!decode_reader_) {
+ mojo::ReportBadMessage(kInvalidStateMessage);
+ return;
+ }
+
+ decode_reader_->Flush(base::BindOnce(&Stream::OnReaderFlushDone,
+ decode_weak_factory_.GetWeakPtr()));
+ }
+
+ void DeinitializeDecoder() {
+ decode_weak_factory_.InvalidateWeakPtrs();
+ decryptor_->DeinitializeDecoder(StreamTypeParam);
+ }
+
+ private:
+ typename Traits::DecodeCallback WrapDecodeCallback(
+ typename Traits::DecodeCallback callback) {
+ if constexpr (StreamTypeParam == StreamType::kAudio) {
+ return mojo::WrapCallbackWithDefaultInvokeIfNotRun(
+ std::move(callback), Status::kError,
+ std::vector<mojom::AudioBufferPtr>());
+ } else {
+ return mojo::WrapCallbackWithDefaultInvokeIfNotRun(
+ std::move(callback), Status::kError, nullptr, mojo::NullRemote());
+ }
+ }
+
+ void OnDecryptReadDone(DecryptCallback callback,
+ mojo::ReportBadMessageCallback bad_message_callback,
+ scoped_refptr<DecoderBuffer> buffer) {
+ if (!buffer) {
+ std::move(callback).Run(Status::kError, nullptr);
+ return;
+ }
+
+ if (!buffer->end_of_stream() && buffer->side_data() &&
+ buffer->side_data()->secure_handle) {
+ std::move(callback).Run(Status::kError, nullptr);
+ std::move(bad_message_callback)
+ .Run("Renderer sent non-zero DecoderBufferSideData.secure_handle.");
+ return;
+ }
+
+ decryptor_->Decrypt(StreamTypeParam, std::move(buffer),
+ base::BindOnce(&Stream::OnDecryptDone,
+ decrypt_weak_factory_.GetWeakPtr(),
+ std::move(callback)));
+ }
+
+ void OnDecryptDone(DecryptCallback callback,
+ Status status,
+ scoped_refptr<DecoderBuffer> buffer) {
+ if (!buffer) {
+ std::move(callback).Run(status, nullptr);
+ return;
+ }
+
+ mojom::DecoderBufferPtr mojo_buffer =
+ decrypt_writer_->WriteDecoderBuffer(std::move(buffer));
+ if (!mojo_buffer) {
+ std::move(callback).Run(Status::kError, nullptr);
+ return;
+ }
+
+ std::move(callback).Run(status, std::move(mojo_buffer));
+ }
+
+ void OnDecoderInitialized(base::OnceCallback<void(bool)> callback,
+ bool success) {
+ std::move(callback).Run(success);
+ }
+
+ void OnDecodeReadDone(mojo::ReportBadMessageCallback bad_message_callback,
+ typename Traits::DecodeCallback callback,
+ scoped_refptr<DecoderBuffer> buffer) {
+ if (!buffer) {
+ Traits::RunDecodeCallbackWithError(std::move(callback));
+ return;
+ }
+
+ if (!buffer->end_of_stream() && buffer->side_data() &&
+ buffer->side_data()->secure_handle) {
+ Traits::RunDecodeCallbackWithError(std::move(callback));
+ std::move(bad_message_callback)
+ .Run("Renderer sent non-zero DecoderBufferSideData.secure_handle.");
+ return;
+ }
+
+ if constexpr (StreamTypeParam == StreamType::kAudio) {
+ decryptor_->DecryptAndDecodeAudio(
+ std::move(buffer), base::BindOnce(&Stream::OnAudioDecoded,
+ decode_weak_factory_.GetWeakPtr(),
+ std::move(callback)));
+ } else {
+ decryptor_->DecryptAndDecodeVideo(
+ std::move(buffer), base::BindOnce(&Stream::OnVideoDecoded,
+ decode_weak_factory_.GetWeakPtr(),
+ std::move(callback)));
+ }
+ }
+
+ void OnAudioDecoded(mojom::Decryptor::DecryptAndDecodeAudioCallback callback,
+ Status status,
+ const media::Decryptor::AudioFrames& frames) {
+ std::vector<mojom::AudioBufferPtr> audio_buffers;
+ for (const auto& frame : frames) {
+ audio_buffers.push_back(mojom::AudioBuffer::From(*frame));
+ }
+ std::move(callback).Run(status, std::move(audio_buffers));
+ }
+
+ void OnVideoDecoded(mojom::Decryptor::DecryptAndDecodeVideoCallback callback,
+ Status status,
+ scoped_refptr<VideoFrame> frame) {
+ if (!frame) {
+ DCHECK_NE(status, Status::kSuccess);
+ std::move(callback).Run(status, nullptr, mojo::NullRemote());
+ return;
+ }
+
+ mojo::PendingRemote<mojom::FrameResourceReleaser> releaser;
+ if (frame->storage_type() == VideoFrame::STORAGE_SHMEM) {
+ mojo::MakeSelfOwnedReceiver(
+ std::make_unique<FrameResourceReleaserImpl>(frame),
+ releaser.InitWithNewPipeAndPassReceiver());
+ }
+
+ std::move(callback).Run(status, std::move(frame), std::move(releaser));
+ }
+
+ void OnReaderFlushDone() { decryptor_->ResetDecoder(StreamTypeParam); }
+
+ const raw_ptr<media::Decryptor> decryptor_;
+ const raw_ptr<MojoDecoderBufferReader> decrypt_reader_;
+ const raw_ptr<MojoDecoderBufferWriter> decrypt_writer_;
+ std::unique_ptr<MojoDecoderBufferReader> decode_reader_;
+
+ base::WeakPtrFactory<Stream> decrypt_weak_factory_{this};
+ base::WeakPtrFactory<Stream> decode_weak_factory_{this};
+};
+
MojoDecryptorService::MojoDecryptorService(
media::Decryptor* decryptor,
std::unique_ptr<CdmContextRef> cdm_context_ref)
: decryptor_(decryptor), cdm_context_ref_(std::move(cdm_context_ref)) {
DVLOG(1) << __func__;
DCHECK(decryptor_);
- // |cdm_context_ref_| could be null, in which case the owner of |this| will
- // make sure |decryptor_| is always valid.
- weak_this_ = weak_factory_.GetWeakPtr();
}
MojoDecryptorService::~MojoDecryptorService() {
@@ -76,20 +319,23 @@
DVLOG(1) << __func__;
if (has_initialize_been_called_) {
- CHECK(mojo::IsInMessageDispatch());
mojo::ReportBadMessage(kInvalidStateMessage);
return;
}
has_initialize_been_called_ = true;
- audio_buffer_reader_ =
- std::make_unique<MojoDecoderBufferReader>(std::move(audio_pipe));
- video_buffer_reader_ =
- std::make_unique<MojoDecoderBufferReader>(std::move(video_pipe));
decrypt_buffer_reader_ =
std::make_unique<MojoDecoderBufferReader>(std::move(decrypt_pipe));
decrypted_buffer_writer_ =
std::make_unique<MojoDecoderBufferWriter>(std::move(decrypted_pipe));
+
+ audio_stream_ = std::make_unique<Stream<StreamType::kAudio>>(
+ decryptor_, decrypt_buffer_reader_.get(), decrypted_buffer_writer_.get());
+ video_stream_ = std::make_unique<Stream<StreamType::kVideo>>(
+ decryptor_, decrypt_buffer_reader_.get(), decrypted_buffer_writer_.get());
+
+ audio_stream_->InitializeDecodePipe(std::move(audio_pipe));
+ video_stream_->InitializeDecodePipe(std::move(video_pipe));
}
void MojoDecryptorService::Decrypt(StreamType stream_type,
@@ -98,297 +344,70 @@
DVLOG(3) << __func__;
if (!decrypt_buffer_reader_) {
- CHECK(mojo::IsInMessageDispatch());
mojo::ReportBadMessage(kInvalidStateMessage);
return;
}
- if (!GetBufferReader(stream_type)) {
- CHECK(mojo::IsInMessageDispatch());
- mojo::ReportBadMessage("Unexpected stream_type");
- return;
+ if (stream_type == StreamType::kAudio) {
+ audio_stream_->Decrypt(std::move(encrypted), std::move(callback));
+ } else {
+ video_stream_->Decrypt(std::move(encrypted), std::move(callback));
}
-
- decrypt_buffer_reader_->ReadDecoderBuffer(
- std::move(encrypted),
- base::BindOnce(&MojoDecryptorService::OnReadDone, weak_this_,
- mojo::GetBadMessageCallback(), stream_type,
- std::move(callback)));
}
void MojoDecryptorService::CancelDecrypt(StreamType stream_type) {
DVLOG(2) << __func__;
- if (!GetBufferReader(stream_type)) {
- CHECK(mojo::IsInMessageDispatch());
- mojo::ReportBadMessage("Unexpected stream_type");
- return;
+ if (stream_type == StreamType::kAudio) {
+ audio_stream_->CancelDecrypt();
+ } else {
+ video_stream_->CancelDecrypt();
}
- decryptor_->CancelDecrypt(stream_type);
}
void MojoDecryptorService::InitializeAudioDecoder(
const AudioDecoderConfig& config,
InitializeAudioDecoderCallback callback) {
DVLOG(1) << __func__;
- decryptor_->InitializeAudioDecoder(
- config, base::BindOnce(&MojoDecryptorService::OnAudioDecoderInitialized,
- weak_this_, std::move(callback)));
+ audio_stream_->InitializeDecoder(config, std::move(callback));
}
void MojoDecryptorService::InitializeVideoDecoder(
... (file diff truncated)
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.ccmedia/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:
- Setup: A compromised renderer acquires a direct
pending_remote<Decryptor>to theMojoDecryptorServicerunning in the sandboxed utility process (sandbox::mojom::Sandbox::kCdm). - Establish Data Pipes: The renderer calls
Decryptor::Initializeand retains the producer ends of the audio/video Mojo data pipes. - Initial Configuration: The renderer calls
InitializeVideoDecoderwith a valid initial configuration (e.g., H.264) and waits for successful completion. - Queue a Stalled Read: The renderer sends a
DecryptAndDecodeVideorequest with an encrypted H.264 buffer but purposely stalls the Mojo data pipe by withholding the buffer payload bytes. On the utility process side,MojoDecoderBufferReaderqueues this read callback (OnVideoRead) and arms its pipe watcher (media/mojo/common/mojo_decoder_buffer_converter.cc). At this point,video_buffer_reader_->HasPendingReads()returnstrue. - Re-initialize Decoder: The renderer calls
InitializeVideoDecoderwith a different configuration (e.g., VP9). BecauseInitializeVideoDecoderlacks aHasPendingReads()check, the request proceeds, causing the underlyingCdmAdapterto instruct the library CDM to re-initialize for VP9. The CDM deallocates previous H.264 decoder contexts and sets up a VP9 context. - 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
DecryptAndDecodeVideocall 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.
References
On This Page