From a2581c3f7c0ee4feec2cfc7d4b8517a375825a6c Mon Sep 17 00:00:00 2001
From: Thomas Guilbert <[email protected]>
Date: Wed, 12 Aug 2026 18:48:50 -0700
Subject: [PATCH] Reject bitstream and EOS inputs in MojoAudioEncoderService

Platform audio encoders operate only on PCM audio. When receiving a
bitstream buffer, `WrapOrCopyToAudioBus()` allocates an uninitialized
AudioBus which could leak uninitialized GPU process heap memory if
subsequently processed as PCM.

This change:
- Rejects bitstream and end-of-stream buffers in
  MojoAudioEncoderService::Encode with kInvalidInputFrame.
- Upgrades DCHECK(!is_bitstream_format_) to CHECK in AudioBus methods
  (CopyAndClipTo, CopyPartialFramesTo, Scale, SwapChannels) to fail
  safely in release builds.
- Replaces empty bitstream buffers with EOS buffers
  during mojom AudioBuffer type conversions.
- Adds test coverage across media_unittests and media_mojo_unittests.

TAG=agy
CONV=52820452-9f78-43b7-a903-e2c9b7911cbe

Bug: 496231550
Change-Id: I9e56cfbb161762670f0ad845f079e7d785ac59b5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8218848
Reviewed-by: Nasko Oskov <[email protected]>
Commit-Queue: Thomas Guilbert <[email protected]>
Reviewed-by: Dale Curtis <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1678514}
---

diff --git a/media/base/audio_buffer_unittest.cc b/media/base/audio_buffer_unittest.cc
index ba15377..4decc900 100644
--- a/media/base/audio_buffer_unittest.cc
+++ b/media/base/audio_buffer_unittest.cc
@@ -965,4 +965,30 @@
   b1 = nullptr;
 }
 
+TEST(AudioBufferTest, WrapOrCopyToAudioBus_BitstreamDecoupledFrameCount) {
+  constexpr ChannelLayout kChannelLayout = CHANNEL_LAYOUT_STEREO;
+  constexpr int kChannelCount = 2;
+  constexpr int kFrameCount = 100000;
+  constexpr uint8_t kTinyData[] = {0x42};
+
+  scoped_refptr<AudioBuffer> buffer = AudioBuffer::CopyBitstreamFrom(
+      kSampleFormatAc3, kChannelLayout, kChannelCount, kSampleRate, kFrameCount,
+      kTinyData, base::Microseconds(1));
+  ASSERT_TRUE(buffer);
+  EXPECT_EQ(kFrameCount, buffer->frame_count());
+  EXPECT_EQ(sizeof(kTinyData), buffer->data_size());
+  EXPECT_TRUE(buffer->IsBitstreamFormat());
+
+  std::unique_ptr<AudioBus> bus = AudioBuffer::WrapOrCopyToAudioBus(buffer);
+  ASSERT_TRUE(bus);
+  EXPECT_TRUE(bus->is_bitstream_format());
+  EXPECT_EQ(static_cast<size_t>(kFrameCount), bus->frames());
+  EXPECT_EQ(sizeof(kTinyData), bus->bitstream_data().size());
+  EXPECT_EQ(kFrameCount, bus->GetBitstreamFrames());
+
+  auto dest = AudioBus::Create(kChannelCount, kFrameCount);
+  EXPECT_DEATH_IF_SUPPORTED(
+      bus->CopyPartialFramesTo(0, kFrameCount, 0, dest.get()), "");
+}
+
 }  // namespace media
diff --git a/media/base/audio_bus.cc b/media/base/audio_bus.cc
index e6a2442..99cdad5e 100644
--- a/media/base/audio_bus.cc
+++ b/media/base/audio_bus.cc
@@ -304,7 +304,7 @@
 }
 
 void AudioBus::CopyAndClipTo(AudioBus* dest) const {
-  DCHECK(!is_bitstream_format_);
+  CHECK(!is_bitstream_format_);
   CHECK_EQ(channels(), dest->channels());
   CHECK_LE(frames(), dest->frames());
   for (auto [src_ch, dest_ch] :
@@ -317,7 +317,7 @@
                                    int frame_count,
                                    int dest_start_frame,
                                    AudioBus* dest) const {
-  DCHECK(!is_bitstream_format_);
+  CHECK(!is_bitstream_format_);
   CHECK_EQ(channels(), dest->channels());
 
   const size_t source_offset = base::checked_cast<size_t>(source_start_frame);
@@ -336,7 +336,7 @@
 }
 
 void AudioBus::Scale(float volume) {
-  DCHECK(!is_bitstream_format_);
+  CHECK(!is_bitstream_format_);
   if (volume > 0 && volume != 1) {
     for (auto channel : channel_data_) {
       vector_math::FMUL(channel, volume, channel);
@@ -347,7 +347,7 @@
 }
 
 void AudioBus::SwapChannels(int a, int b) {
-  DCHECK(!is_bitstream_format_);
+  CHECK(!is_bitstream_format_);
   DCHECK(a < channels() && a >= 0);
   DCHECK(b < channels() && b >= 0);
   DCHECK_NE(a, b);
diff --git a/media/mojo/clients/mojo_audio_encoder_unittest.cc b/media/mojo/clients/mojo_audio_encoder_unittest.cc
index 390437e40..1f0b2d3 100644
--- a/media/mojo/clients/mojo_audio_encoder_unittest.cc
+++ b/media/mojo/clients/mojo_audio_encoder_unittest.cc
@@ -30,6 +30,7 @@
 #include "mojo/public/cpp/bindings/pending_receiver.h"
 #include "mojo/public/cpp/bindings/pending_remote.h"
 #include "mojo/public/cpp/bindings/self_owned_receiver.h"
+#include "mojo/public/cpp/system/functions.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 using ::base::test::RunOnceCallback;
@@ -41,6 +42,19 @@
 
 namespace media {
 
+namespace {
+
+AudioEncoder::Options MakeOptions() {
+  AudioEncoder::Options options;
+  options.codec = AudioCodec::kOpus;
+  options.bitrate = 128000;
+  options.channels = 2;
+  options.sample_rate = 44000;
+  return options;
+}
+
+}  // namespace
+
 // Tests MojoAudioEncoder (client) and MojoAudioEncoderService (service).
 class MojoAudioEncoderTest : public ::testing::Test {
  public:
@@ -95,15 +109,6 @@
     return result;
   }
 
-  AudioEncoder::Options MakeOptions() {
-    AudioEncoder::Options options;
-    options.codec = AudioCodec::kOpus;
-    options.bitrate = 128000;
-    options.channels = 2;
-    options.sample_rate = 44000;
-    return options;
-  }
-
   base::TimeTicks FromMilliseconds(int ms) {
     return base::TimeTicks() + base::Milliseconds(ms);
   }
@@ -475,4 +480,65 @@
   EXPECT_EQ(error_count, input_count);
 }
 
+TEST(MojoAudioEncoderServiceTest, ServiceRejectsInvalidInputs) {
+  base::test::SingleThreadTaskEnvironment task_environment;
+
+  // Helper to initialize a fresh service instance and verify that sending an
+  // invalid buffer triggers a mojo bad message report.
+  auto test_invalid_buffer = [&](mojom::AudioBufferPtr buffer) {
+    // Boilerplate service setup and initialization.
+    auto mock_encoder = std::make_unique<StrictMock<MockAudioEncoder>>();
+    EXPECT_CALL(*mock_encoder, Initialize(_, _, _))
+        .WillOnce(RunOnceCallback<2>(EncoderStatus::Codes::kOk));
+    EXPECT_CALL(*mock_encoder, OnDestruct());
+
+    auto service =
+        std::make_unique<MojoAudioEncoderService>(std::move(mock_encoder));
+    mojo::Remote<mojom::AudioEncoder> remote;
+    auto receiver = std::make_unique<mojo::Receiver<mojom::AudioEncoder>>(
+        service.get(), remote.BindNewPipeAndPassReceiver());
+
+    mojo::PendingAssociatedRemote<mojom::AudioEncoderClient> client_remote;
+    auto client_receiver = client_remote.InitWithNewEndpointAndPassReceiver();
+    base::RunLoop init_loop;
+    remote->Initialize(std::move(client_remote), MakeOptions(),
+                       base::BindLambdaForTesting([&](const EncoderStatus& s) {
+                         EXPECT_TRUE(s.is_ok());
+                         init_loop.Quit();
+                       }));
+    init_loop.Run();
+
+    // Intercept bad message reports.
+    bool bad_message_received = false;
+    base::RunLoop bad_message_loop;
+    mojo::SetDefaultProcessErrorHandler(
+        base::BindLambdaForTesting([&](const std::string& error) {
+          bad_message_received = true;
+          bad_message_loop.Quit();
+        }));
+
+    // Send the invalid buffer and verify a bad message is reported.
+    remote->Encode(std::move(buffer), base::DoNothing());
+    bad_message_loop.Run();
+    EXPECT_TRUE(bad_message_received);

diff --git a/media/base/audio_buffer_unittest.cc b/media/base/audio_buffer_unittest.cc
index ba15377..4decc900 100644
--- a/media/base/audio_buffer_unittest.cc
+++ b/media/base/audio_buffer_unittest.cc
@@ -965,4 +965,30 @@
   b1 = nullptr;
 }
 
+TEST(AudioBufferTest, WrapOrCopyToAudioBus_BitstreamDecoupledFrameCount) {
+  constexpr ChannelLayout kChannelLayout = CHANNEL_LAYOUT_STEREO;
+  constexpr int kChannelCount = 2;
+  constexpr int kFrameCount = 100000;
+  constexpr uint8_t kTinyData[] = {0x42};
+
+  scoped_refptr<AudioBuffer> buffer = AudioBuffer::CopyBitstreamFrom(
+      kSampleFormatAc3, kChannelLayout, kChannelCount, kSampleRate, kFrameCount,
+      kTinyData, base::Microseconds(1));
+  ASSERT_TRUE(buffer);
+  EXPECT_EQ(kFrameCount, buffer->frame_count());
+  EXPECT_EQ(sizeof(kTinyData), buffer->data_size());
+  EXPECT_TRUE(buffer->IsBitstreamFormat());
+
+  std::unique_ptr<AudioBus> bus = AudioBuffer::WrapOrCopyToAudioBus(buffer);
+  ASSERT_TRUE(bus);
+  EXPECT_TRUE(bus->is_bitstream_format());
+  EXPECT_EQ(static_cast<size_t>(kFrameCount), bus->frames());
+  EXPECT_EQ(sizeof(kTinyData), bus->bitstream_data().size());
+  EXPECT_EQ(kFrameCount, bus->GetBitstreamFrames());
+
+  auto dest = AudioBus::Create(kChannelCount, kFrameCount);
+  EXPECT_DEATH_IF_SUPPORTED(
+      bus->CopyPartialFramesTo(0, kFrameCount, 0, dest.get()), "");
+}
+
 }  // namespace media
diff --git a/media/mojo/clients/mojo_audio_encoder_unittest.cc b/media/mojo/clients/mojo_audio_encoder_unittest.cc
index 390437e40..1f0b2d3 100644
--- a/media/mojo/clients/mojo_audio_encoder_unittest.cc
+++ b/media/mojo/clients/mojo_audio_encoder_unittest.cc
@@ -30,6 +30,7 @@
 #include "mojo/public/cpp/bindings/pending_receiver.h"
 #include "mojo/public/cpp/bindings/pending_remote.h"
 #include "mojo/public/cpp/bindings/self_owned_receiver.h"
+#include "mojo/public/cpp/system/functions.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 using ::base::test::RunOnceCallback;
@@ -41,6 +42,19 @@
 
 namespace media {
 
+namespace {
+
+AudioEncoder::Options MakeOptions() {
+  AudioEncoder::Options options;
+  options.codec = AudioCodec::kOpus;
+  options.bitrate = 128000;
+  options.channels = 2;
+  options.sample_rate = 44000;
+  return options;
+}
+
+}  // namespace
+
 // Tests MojoAudioEncoder (client) and MojoAudioEncoderService (service).
 class MojoAudioEncoderTest : public ::testing::Test {
  public:
@@ -95,15 +109,6 @@
     return result;
   }
 
-  AudioEncoder::Options MakeOptions() {
-    AudioEncoder::Options options;
-    options.codec = AudioCodec::kOpus;
-    options.bitrate = 128000;
-    options.channels = 2;
-    options.sample_rate = 44000;
-    return options;
-  }
-
   base::TimeTicks FromMilliseconds(int ms) {
     return base::TimeTicks() + base::Milliseconds(ms);
   }
@@ -475,4 +480,65 @@
   EXPECT_EQ(error_count, input_count);
 }
 
+TEST(MojoAudioEncoderServiceTest, ServiceRejectsInvalidInputs) {
+  base::test::SingleThreadTaskEnvironment task_environment;
+
+  // Helper to initialize a fresh service instance and verify that sending an
+  // invalid buffer triggers a mojo bad message report.
+  auto test_invalid_buffer = [&](mojom::AudioBufferPtr buffer) {
+    // Boilerplate service setup and initialization.
+    auto mock_encoder = std::make_unique<StrictMock<MockAudioEncoder>>();
+    EXPECT_CALL(*mock_encoder, Initialize(_, _, _))
+        .WillOnce(RunOnceCallback<2>(EncoderStatus::Codes::kOk));
+    EXPECT_CALL(*mock_encoder, OnDestruct());
+
+    auto service =
+        std::make_unique<MojoAudioEncoderService>(std::move(mock_encoder));
+    mojo::Remote<mojom::AudioEncoder> remote;
+    auto receiver = std::make_unique<mojo::Receiver<mojom::AudioEncoder>>(
+        service.get(), remote.BindNewPipeAndPassReceiver());
+
+    mojo::PendingAssociatedRemote<mojom::AudioEncoderClient> client_remote;
+    auto client_receiver = client_remote.InitWithNewEndpointAndPassReceiver();
+    base::RunLoop init_loop;
+    remote->Initialize(std::move(client_remote), MakeOptions(),
+                       base::BindLambdaForTesting([&](const EncoderStatus& s) {
+                         EXPECT_TRUE(s.is_ok());
+                         init_loop.Quit();
+                       }));
+    init_loop.Run();
+
+    // Intercept bad message reports.
+    bool bad_message_received = false;
+    base::RunLoop bad_message_loop;
+    mojo::SetDefaultProcessErrorHandler(
+        base::BindLambdaForTesting([&](const std::string& error) {
+          bad_message_received = true;
+          bad_message_loop.Quit();
+        }));
+
+    // Send the invalid buffer and verify a bad message is reported.
+    remote->Encode(std::move(buffer), base::DoNothing());
+    bad_message_loop.Run();
+    EXPECT_TRUE(bad_message_received);
+
+    mojo::SetDefaultProcessErrorHandler(base::NullCallback());
+  };
+
+  // Test bitstream format input.
+  auto bitstream_buffer = mojom::AudioBuffer::New();
+  bitstream_buffer->sample_format = SampleFormat::kSampleFormatAc3;
+  bitstream_buffer->channel_layout = CHANNEL_LAYOUT_STEREO;
+  bitstream_buffer->channel_count = 2;
+  bitstream_buffer->sample_rate = 44000;
+  bitstream_buffer->frame_count = 100;
+  bitstream_buffer->data = std::vector<uint8_t>(10);
+  test_invalid_buffer(std::move(bitstream_buffer));
+
+  // Test End-Of-Stream input.
+  auto eos_buffer = mojom::AudioBuffer::New();
+  eos_buffer->end_of_stream = true;
+  test_invalid_buffer(std::move(eos_buffer));
+}
+
 }  // namespace media
diff --git a/media/mojo/common/media_type_converters_unittest.cc b/media/mojo/common/media_type_converters_unittest.cc
index 34cb11b1..c8e1a8cb 100644
--- a/media/mojo/common/media_type_converters_unittest.cc
+++ b/media/mojo/common/media_type_converters_unittest.cc
@@ -387,4 +387,20 @@
   EXPECT_FALSE(decrypt_config);
 }
 
+TEST(MediaTypeConvertersTest, ConvertAudioBuffer_BitstreamEmptyData) {
+  auto ptr = mojom::AudioBuffer::New();
+  ptr->sample_format = SampleFormat::kSampleFormatAc3;
+  ptr->channel_layout = CHANNEL_LAYOUT_STEREO;
+  ptr->channel_count = 2;
+  ptr->sample_rate = 48000;
+  ptr->frame_count = 100;
+  // Explicitly provide empty data for a bitstream buffer.
+  ptr->data = std::vector<uint8_t>();
+
+  auto result = ptr.To<scoped_refptr<AudioBuffer>>();
+  // Bitstream buffers with empty data should be replaced with EOS.
+  ASSERT_TRUE(result);
+  EXPECT_TRUE(result->end_of_stream());
+}
+
 }  // namespace media