From d0a80fe50c1e778fdb9e2d5283e7e5ad193bffcd Mon Sep 17 00:00:00 2001
From: Eugene Zemtsov <[email protected]>
Date: Mon, 02 Mar 2026 17:17:06 -0800
Subject: [PATCH] media: Prevent passing of frames with absurdly large strides via Mojo

- Make strides unsigned in mojo, as they are in VideoFrame
- Validate that each plane's footprint (offset + stride * rows)
    fits in the buffer.
- Remove support for interleaved planes. IMC4 pixel format.

Bug: 484547633, 378046071
Change-Id: I8e5dbebddc434041bd7c31c2b16c2b5963314061
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7614704
Reviewed-by: Xiaohan Wang <[email protected]>
Reviewed-by: Dale Curtis <[email protected]>
Commit-Queue: Eugene Zemtsov <[email protected]>
Reviewed-by: Matthew Denton <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1592880}
---

diff --git a/media/base/video_frame_layout.cc b/media/base/video_frame_layout.cc
index 5120fc36..3d6ff38f 100644
--- a/media/base/video_frame_layout.cc
+++ b/media/base/video_frame_layout.cc
@@ -190,7 +190,8 @@
     return false;
   }
 
-  for (const auto& plane : planes_) {
+  for (size_t plane_idx = 0; plane_idx < planes_.size(); ++plane_idx) {
+    const auto& plane = planes_[plane_idx];
     if (plane.offset > data_size || plane.size > data_size) {
       return false;
     }
@@ -201,6 +202,14 @@
     if (!plane_end.IsValid() || plane_end.ValueOrDie() > data_size) {
       return false;
     }
+
+    size_t rows = VideoFrame::Rows(plane_idx, format_, coded_size_.height());
+    // Offset + stride * rows: furthermost byte that can be reasonably read
+    // during copying or conversion of the plane.
+    auto read_end = base::CheckMul(plane.stride, rows) + plane.offset;
+    if (!read_end.IsValid() || read_end.ValueOrDie() > data_size) {
+      return false;
+    }
   }
 
   return true;
diff --git a/media/base/video_frame_layout_unittest.cc b/media/base/video_frame_layout_unittest.cc
index 7b83496..e4351ab 100644
--- a/media/base/video_frame_layout_unittest.cc
+++ b/media/base/video_frame_layout_unittest.cc
@@ -318,8 +318,8 @@
   auto coded_size = gfx::Size(320, 180);
 
   std::vector<size_t> strides = {384, 192, 192};
-  std::vector<size_t> offsets = {0, 200, 300};
-  std::vector<size_t> sizes = {200, 100, 100};
+  std::vector<size_t> offsets = {0, 70000, 90000};
+  std::vector<size_t> sizes = {70000, 20000, 20000};
   std::vector<ColorPlaneLayout> planes(strides.size());
   for (size_t i = 0; i < strides.size(); i++) {
     planes[i].stride = strides[i];
diff --git a/media/mojo/mojom/media_types.mojom b/media/mojo/mojom/media_types.mojom
index d5e2654..1b88d88 100644
--- a/media/mojo/mojom/media_types.mojom
+++ b/media/mojo/mojom/media_types.mojom
@@ -488,7 +488,7 @@
 
   // Stride and offsets for each plane. Offsets are relative to the start
   // of |frame_data|.
-  array<int32> strides;
+  array<uint32> strides;
   array<uint32> offsets;
 };
 
diff --git a/media/mojo/mojom/video_frame_mojom_traits.cc b/media/mojo/mojom/video_frame_mojom_traits.cc
index 058a12b4..e97b860 100644
--- a/media/mojo/mojom/video_frame_mojom_traits.cc
+++ b/media/mojo/mojom/video_frame_mojom_traits.cc
@@ -39,7 +39,7 @@
 
 base::ReadOnlySharedMemoryRegion CreateRegion(const media::VideoFrame& frame,
                                               std::vector<uint32_t>& offsets,
-                                              std::vector<int32_t>& strides) {
+                                              std::vector<uint32_t>& strides) {
   TRACE_EVENT0("media", "VideoFrameDataPtr::CreateRegion");
   size_t num_planes = media::VideoFrame::NumPlanes(frame.format());
   DCHECK_LE(num_planes, 3u);
@@ -105,7 +105,7 @@
       input->storage_type() == media::VideoFrame::STORAGE_UNOWNED_MEMORY ||
       input->storage_type() == media::VideoFrame::STORAGE_OWNED_MEMORY) {
     std::vector<uint32_t> offsets;
-    std::vector<int32_t> strides;
+    std::vector<uint32_t> strides;
     auto region = CreateRegion(*input, offsets, strides);
     if (!region.IsValid()) {
       DLOG(ERROR) << "Failed to create region from VideoFrame";
@@ -272,7 +272,7 @@
     mojo::ArrayDataView<uint32_t> offsets;
     shared_memory_data.GetOffsetsDataView(&offsets);
 
-    mojo::ArrayDataView<int32_t> strides;
+    mojo::ArrayDataView<uint32_t> strides;
     shared_memory_data.GetStridesDataView(&strides);
 
     base::ReadOnlySharedMemoryMapping mapping = region.Map();
@@ -288,7 +288,6 @@
     }
 
     auto mapped_region = mapping.GetMemoryAsSpan<uint8_t>();
-    std::array<base::span<const uint8_t>, 3> plane_data;
     std::vector<media::ColorPlaneLayout> planes(num_planes);
     for (size_t i = 0; i < num_planes; i++) {
       if (offsets[i] > mapped_region.size()) {
@@ -300,16 +299,8 @@
 
       planes[i].stride = strides[i];
       planes[i].offset = base::strict_cast<size_t>(offsets[i]);
-      const size_t space_till_mapping_end = mapping.size() - offsets[i];
-      const size_t calculated_plane_size =
+      planes[i].size =
           media::VideoFrame::Rows(i, format, coded_size.height()) * strides[i];
-
-      // TODO(crbug.com/378046071) For H.264 content Widevine outputs planes
-      // in IMC4 pixel format. Since Y and V planes in IMC4 overlap,
-      // the distance to the next plane can't be used to determent the size of
-      // the current plane.
-      planes[i].size = std::min(calculated_plane_size, space_till_mapping_end);
-      plane_data[i] = mapped_region.subspan(offsets[i], planes[i].size);
     }
 
     auto layout = media::VideoFrameLayout::CreateWithPlanes(format, coded_size,
@@ -319,6 +310,12 @@
       return false;
     }
 
+    std::array<base::span<const uint8_t>, 3> plane_data;
+    for (size_t i = 0; i < num_planes; i++) {
+      plane_data[i] = mapped_region.subspan(layout->planes()[i].offset,
+                                            layout->planes()[i].size);
+    }
+
     if (media::IsYuvPlanar(format) && media::IsOpaque(format)) {
       frame = media::VideoFrame::WrapExternalYuvDataWithLayout(
           *layout, visible_rect, natural_size, plane_data[0], plane_data[1],
diff --git a/media/mojo/mojom/video_frame_mojom_traits_unittest.cc b/media/mojo/mojom/video_frame_mojom_traits_unittest.cc
index b5c92fd..755fade 100644
--- a/media/mojo/mojom/video_frame_mojom_traits_unittest.cc
+++ b/media/mojo/mojom/video_frame_mojom_traits_unittest.cc
@@ -195,79 +195,6 @@
   }
 }
 
-TEST_F(VideoFrameStructTraitsTest, InterleavedPlanes) {
-  constexpr VideoFrame::StorageType storage_type = VideoFrame::STORAGE_SHMEM;
-  constexpr VideoPixelFormat format = PIXEL_FORMAT_I420;
-  constexpr gfx::Size kCodedSize(100, 100);
-  constexpr gfx::Rect kVisibleRect(kCodedSize);
-  constexpr gfx::Size kNaturalSize = kCodedSize;
-  constexpr base::TimeDelta kTimestamp;
-
-  scoped_refptr<media::VideoFrame> frame;
-
-  std::vector<size_t> strides = VideoFrame::ComputeStrides(format, kCodedSize);
-  ASSERT_EQ(strides[1], strides[2]);
-
-  size_t aggregate_size = 0;
-  std::array<size_t, 3> sizes = {};
-  for (size_t i = 0; i < strides.size(); ++i) {
-    sizes[i] =
-        media::VideoFrame::Rows(i, format, kCodedSize.height()) * strides[i];
-    aggregate_size += sizes[i];
-  }
-  auto region = base::WritableSharedMemoryRegion::Create(aggregate_size);
-  ASSERT_TRUE(region.IsValid());
-  auto mapping = region.MapAt(0, aggregate_size);
-
-  auto [y_plane, uv_plane] =
-      mapping.GetMemoryAsSpan<uint8_t>().split_at(sizes[0]);
-  std::ranges::fill(y_plane, 1);
-
-  // Setup memory layout where U and V planes occupy the same space, but have
-  // interleaving U and V rows. This is achieved by doubling the stride.
-  size_t normal_stride = strides[1];
-  size_t uv_stride = normal_stride * 2;
-
-  int yu_rows = media::VideoFrame::Rows(1, format, kCodedSize.height());
-  auto uv_plane2 = uv_plane;  // Loop below is destructive.
-  for (int i = 0; i < yu_rows; ++i) {
-    const auto [u, v] = uv_plane2.take_first(uv_stride).split_at(normal_stride);
-    std::ranges::fill(u, 2);
-    std::ranges::fill(v, 3);
-  }
-
-  frame = media::VideoFrame::WrapExternalYuvData(
-      format, kCodedSize, kVisibleRect, kNaturalSize, strides[0], uv_stride,
-      uv_stride, y_plane, uv_plane, uv_plane.subspan(normal_stride),
-      kTimestamp);

diff --git a/media/base/video_frame_layout_unittest.cc b/media/base/video_frame_layout_unittest.cc
index 7b83496..e4351ab 100644
--- a/media/base/video_frame_layout_unittest.cc
+++ b/media/base/video_frame_layout_unittest.cc
@@ -318,8 +318,8 @@
   auto coded_size = gfx::Size(320, 180);
 
   std::vector<size_t> strides = {384, 192, 192};
-  std::vector<size_t> offsets = {0, 200, 300};
-  std::vector<size_t> sizes = {200, 100, 100};
+  std::vector<size_t> offsets = {0, 70000, 90000};
+  std::vector<size_t> sizes = {70000, 20000, 20000};
   std::vector<ColorPlaneLayout> planes(strides.size());
   for (size_t i = 0; i < strides.size(); i++) {
     planes[i].stride = strides[i];
diff --git a/media/mojo/mojom/video_frame_mojom_traits_unittest.cc b/media/mojo/mojom/video_frame_mojom_traits_unittest.cc
index b5c92fd..755fade 100644
--- a/media/mojo/mojom/video_frame_mojom_traits_unittest.cc
+++ b/media/mojo/mojom/video_frame_mojom_traits_unittest.cc
@@ -195,79 +195,6 @@
   }
 }
 
-TEST_F(VideoFrameStructTraitsTest, InterleavedPlanes) {
-  constexpr VideoFrame::StorageType storage_type = VideoFrame::STORAGE_SHMEM;
-  constexpr VideoPixelFormat format = PIXEL_FORMAT_I420;
-  constexpr gfx::Size kCodedSize(100, 100);
-  constexpr gfx::Rect kVisibleRect(kCodedSize);
-  constexpr gfx::Size kNaturalSize = kCodedSize;
-  constexpr base::TimeDelta kTimestamp;
-
-  scoped_refptr<media::VideoFrame> frame;
-
-  std::vector<size_t> strides = VideoFrame::ComputeStrides(format, kCodedSize);
-  ASSERT_EQ(strides[1], strides[2]);
-
-  size_t aggregate_size = 0;
-  std::array<size_t, 3> sizes = {};
-  for (size_t i = 0; i < strides.size(); ++i) {
-    sizes[i] =
-        media::VideoFrame::Rows(i, format, kCodedSize.height()) * strides[i];
-    aggregate_size += sizes[i];
-  }
-  auto region = base::WritableSharedMemoryRegion::Create(aggregate_size);
-  ASSERT_TRUE(region.IsValid());
-  auto mapping = region.MapAt(0, aggregate_size);
-
-  auto [y_plane, uv_plane] =
-      mapping.GetMemoryAsSpan<uint8_t>().split_at(sizes[0]);
-  std::ranges::fill(y_plane, 1);
-
-  // Setup memory layout where U and V planes occupy the same space, but have
-  // interleaving U and V rows. This is achieved by doubling the stride.
-  size_t normal_stride = strides[1];
-  size_t uv_stride = normal_stride * 2;
-
-  int yu_rows = media::VideoFrame::Rows(1, format, kCodedSize.height());
-  auto uv_plane2 = uv_plane;  // Loop below is destructive.
-  for (int i = 0; i < yu_rows; ++i) {
-    const auto [u, v] = uv_plane2.take_first(uv_stride).split_at(normal_stride);
-    std::ranges::fill(u, 2);
-    std::ranges::fill(v, 3);
-  }
-
-  frame = media::VideoFrame::WrapExternalYuvData(
-      format, kCodedSize, kVisibleRect, kNaturalSize, strides[0], uv_stride,
-      uv_stride, y_plane, uv_plane, uv_plane.subspan(normal_stride),
-      kTimestamp);
-  auto ro_region =
-      base::WritableSharedMemoryRegion::ConvertToReadOnly(std::move(region));
-  frame->BackWithSharedMemory(&ro_region);
-
-  EXPECT_TRUE(frame);
-  EXPECT_EQ(frame->storage_type(), storage_type);
-  EXPECT_TRUE(RoundTrip(&frame));
-  EXPECT_TRUE(frame);
-  EXPECT_EQ(frame->format(), format);
-  EXPECT_EQ(frame->coded_size(), kCodedSize);
-
-  auto plane_1 = frame->GetVisiblePlaneData(1);
-  auto plane_2 = frame->GetVisiblePlaneData(2);
-  // Bytes between the visible edge and the full stride are not considered part
-  // of the visible plane, and may not be accessible through the above spans.
-  const size_t row_bytes_1 =
-      VideoFrame::RowBytes(1, format, kCodedSize.width());
-  const size_t row_bytes_2 =
-      VideoFrame::RowBytes(2, format, kCodedSize.width());
-  for (int i = 0; i < yu_rows; ++i) {
-    const auto [u, v] = uv_plane.take_first(uv_stride).split_at(normal_stride);
-    EXPECT_EQ(plane_1.subspan(i * frame->stride(1), row_bytes_1),
-              u.first(row_bytes_1));
-    EXPECT_EQ(plane_2.subspan(i * frame->stride(2), row_bytes_2),
-              v.first(row_bytes_2));
-  }
-}
-
 TEST_F(VideoFrameStructTraitsTest, InvalidOffsets) {
   constexpr auto kFormat = PIXEL_FORMAT_I420;