CVE-2026-11089
Overview
Changed Functions
| Function | Change | Notes |
|---|---|---|
ifmedia/base/test_helpers.cc |
modified |
Files Changed
media/base/test_helpers.ccmedia/base/video_frame.ccmedia/base/video_frame.hmedia/base/video_frame_converter_internals.cc
Patch
From 734aa712b00e8c103715a3b9c3fe687d1ff851d6 Mon Sep 17 00:00:00 2001 From: Dale Curtis <[email protected]> Date: Mon, 13 Apr 2026 17:41:50 -0700 Subject: [PATCH] Fix odd sized frame handling and add tests for VideoFrameConverter VFC was skipping the last pixel of each row during conversion of odd sized frames. The fix is to use the subsample aligned width and height values. Tests which fail before the fix and pass after have been added. R=eugene Fixed: 500154880 Change-Id: I790e550932f680ac9ce4482339e97071d56ff418 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7737851 Commit-Queue: Dale Curtis <[email protected]> Reviewed-by: Eugene Zemtsov <[email protected]> Cr-Commit-Position: refs/heads/main@{#1614100} --- diff --git a/media/base/test_helpers.cc b/media/base/test_helpers.cc index 75b80d2..f8e8fe2e 100644 --- a/media/base/test_helpers.cc +++ b/media/base/test_helpers.cc @@ -134,27 +134,31 @@ std::tie(yellow, red, blue, green) = FourColors(IsOpaque(dest_frame.format()), xor_mask); + const int half_width = (visible_size.width() / 2) & ~1; + const int half_height = (visible_size.height() / 2) & ~1; + const int remaining_width = visible_size.width() - half_width; + const int remaining_height = visible_size.height() - half_height; + uint8_t y, u, v, a; // Yellow top left. std::tie(y, u, v, a) = RGBToYUV(yellow); - I4xxxRect(output_frame, 0, 0, visible_size.width() / 2, - visible_size.height() / 2, y, u, v, a); + I4xxxRect(output_frame, 0, 0, half_width, half_height, y, u, v, a); // Red top right. std::tie(y, u, v, a) = RGBToYUV(red); - I4xxxRect(output_frame, visible_size.width() / 2, 0, visible_size.width() / 2, - visible_size.height() / 2, y, u, v, a); + I4xxxRect(output_frame, half_width, 0, remaining_width, half_height, y, u, v, + a); // Blue bottom left. std::tie(y, u, v, a) = RGBToYUV(blue); - I4xxxRect(output_frame, 0, visible_size.height() / 2, - visible_size.width() / 2, visible_size.height() / 2, y, u, v, a); + I4xxxRect(output_frame, 0, half_height, half_width, remaining_height, y, u, v, + a); // Green bottom right. std::tie(y, u, v, a) = RGBToYUV(green); - I4xxxRect(output_frame, visible_size.width() / 2, visible_size.height() / 2, - visible_size.width() / 2, visible_size.height() / 2, y, u, v, a); + I4xxxRect(output_frame, half_width, half_height, remaining_width, + remaining_height, y, u, v, a); if (temp_frame) { ASSERT_EQ(libyuv::I420ToNV12( @@ -198,35 +202,37 @@ std::tie(yellow, red, blue, green) = FourColors(IsOpaque(dest_frame.format()), xor_mask); + const int half_width = (visible_size.width() / 2) & ~1; + const int half_height = (visible_size.height() / 2) & ~1; + const int remaining_width = visible_size.width() - half_width; + const int remaining_height = visible_size.height() - half_height; + // Yellow top left. ASSERT_EQ(libyuv::ARGBRect( dest_frame.GetWritableVisibleData(VideoFrame::Plane::kARGB), - dest_frame.stride(VideoFrame::Plane::kARGB), 0, 0, - visible_size.width() / 2, visible_size.height() / 2, yellow), + dest_frame.stride(VideoFrame::Plane::kARGB), 0, 0, half_width, + half_height, yellow), 0); // Red top right. - ASSERT_EQ( - libyuv::ARGBRect( - dest_frame.GetWritableVisibleData(VideoFrame::Plane::kARGB), - dest_frame.stride(VideoFrame::Plane::kARGB), visible_size.width() / 2, - 0, visible_size.width() / 2, visible_size.height() / 2, red), - 0); + ASSERT_EQ(libyuv::ARGBRect( + dest_frame.GetWritableVisibleData(VideoFrame::Plane::kARGB), + dest_frame.stride(VideoFrame::Plane::kARGB), half_width, 0, + remaining_width, half_height, red), + 0); // Blue bottom left. ASSERT_EQ(libyuv::ARGBRect( dest_frame.GetWritableVisibleData(VideoFrame::Plane::kARGB), - dest_frame.stride(VideoFrame::Plane::kARGB), 0, - visible_size.height() / 2, visible_size.width() / 2, - visible_size.height() / 2, blue), + dest_frame.stride(VideoFrame::Plane::kARGB), 0, half_height, + half_width, remaining_height, blue), 0); // Green bottom right. ASSERT_EQ(libyuv::ARGBRect( dest_frame.GetWritableVisibleData(VideoFrame::Plane::kARGB), - dest_frame.stride(VideoFrame::Plane::kARGB), - visible_size.width() / 2, visible_size.height() / 2, - visible_size.width() / 2, visible_size.height() / 2, green), + dest_frame.stride(VideoFrame::Plane::kARGB), half_width, + half_height, remaining_width, remaining_height, green), 0); if (dest_frame.format() == PIXEL_FORMAT_XBGR || diff --git a/media/base/video_frame.cc b/media/base/video_frame.cc index 952e57d..ec232e5 100644 --- a/media/base/video_frame.cc +++ b/media/base/video_frame.cc @@ -1267,6 +1267,10 @@ return Rows(plane, format(), coded_size().height()); } +int VideoFrame::columns(size_t plane) const { + return Columns(plane, format(), coded_size().width()); +} + int VideoFrame::GetVisibleRowBytes(size_t plane) const { return RowBytes(plane, format(), visible_rect().width()); } @@ -1275,8 +1279,8 @@ return Rows(plane, format(), visible_rect().height()); } -int VideoFrame::columns(size_t plane) const { - return Columns(plane, format(), coded_size().width()); +int VideoFrame::GetVisibleColumns(size_t plane) const { + return Columns(plane, format(), visible_rect().width()); } template <typename T> diff --git a/media/base/video_frame.h b/media/base/video_frame.h index 378c869..4929bcd 100644 --- a/media/base/video_frame.h +++ b/media/base/video_frame.h @@ -543,13 +543,15 @@ int row_bytes(size_t plane) const; int rows(size_t plane) const; - // Similar to row_bytes() and rows(), but instead refers to the visible area. - int GetVisibleRowBytes(size_t plane) const; - int GetVisibleRows(size_t plane) const; - // Returns the number of columns for a given plane. int columns(size_t plane) const; + // Similar to row_bytes(), rows(), and columns(), but instead refers to the + // visible area. + int GetVisibleColumns(size_t plane) const; + int GetVisibleRowBytes(size_t plane) const; + int GetVisibleRows(size_t plane) const; + // Returns pointer to the buffer for a given plane, if HasDirectCpuAccess() is // true. The memory is owned by VideoFrame object and must not be freed by the // caller. diff --git a/media/base/video_frame_converter_internals.cc b/media/base/video_frame_converter_internals.cc index ab17fde..b9b80be 100644 --- a/media/base/video_frame_converter_internals.cc +++ b/media/base/video_frame_converter_internals.cc @@ -167,18 +167,12 @@ : libyuv::kFilterBox; for (size_t i = 0; i < VideoFrame::NumPlanes(dest_frame.format()); ++i) { - libyuv::ScalePlane( - src_frame.visible_data(i), src_frame.stride(i), - VideoFrame::Columns(i, src_frame.format(), - src_frame.visible_rect().size().width()), - VideoFrame::Rows(i, src_frame.format(), - src_frame.visible_rect().size().height()), - dest_frame.GetWritableVisibleData(i), dest_frame.stride(i), - VideoFrame::Columns(i, dest_frame.format(), - dest_frame.visible_rect().size().width()), - VideoFrame::Rows(i, dest_frame.format(), - dest_frame.visible_rect().size().height()), - kDefaultFiltering); + libyuv::ScalePlane(src_frame.visible_data(i), src_frame.stride(i), + src_frame.GetVisibleColumns(i), + src_frame.GetVisibleRows(i), + dest_frame.GetWritableVisibleData(i), + dest_frame.stride(i), dest_frame.GetVisibleColumns(i), + dest_frame.GetVisibleRows(i), kDefaultFiltering); } } @@ -261,8 +255,8 @@ src_frame.stride(VideoFrame::Plane::kV), dest_frame.GetWritableVisibleData(VideoFrame::Plane::kUV), dest_frame.stride(VideoFrame::Plane::kUV),
Regression Test / PoC
diff --git a/media/base/video_frame_converter_unittest.cc b/media/base/video_frame_converter_unittest.cc
index ac6d2fbc..df88fb12 100644
--- a/media/base/video_frame_converter_unittest.cc
+++ b/media/base/video_frame_converter_unittest.cc
@@ -20,14 +20,54 @@
// calculate the SSIM for smaller sizes.
constexpr gfx::Size kCodedSize(128, 128);
constexpr gfx::Rect kVisibleRect(64, 64, 64, 64);
+constexpr gfx::Rect kOddRect(0, 0, 63, 63);
-gfx::Size SelectDestSize(bool scaled) {
- return scaled ? gfx::ScaleToRoundedSize(kCodedSize, 0.5) : kCodedSize;
+enum class TestConversionType {
+ kNormal,
+ kScaled,
+ kOdd, // Visible rect is the same as the coded size but odd.
+};
+
+gfx::Size SelectDestSize(TestConversionType conversion_type) {
+ switch (conversion_type) {
+ case TestConversionType::kNormal:
+ return kCodedSize;
+ case TestConversionType::kScaled:
+ return gfx::ScaleToRoundedSize(kCodedSize, 0.5);
+ case TestConversionType::kOdd:
+ return kOddRect.size();
+ }
}
-gfx::Rect SelectDestRect(bool scaled) {
- return scaled ? gfx::ScaleToRoundedRect(kVisibleRect, 0.5, 0.5)
- : kVisibleRect;
+gfx::Rect SelectDestRect(TestConversionType conversion_type) {
+ switch (conversion_type) {
+ case TestConversionType::kNormal:
+ return kVisibleRect;
+ case TestConversionType::kScaled:
+ return gfx::ScaleToRoundedRect(kVisibleRect, 0.5, 0.5);
+ case TestConversionType::kOdd:
+ return kOddRect;
+ }
+}
+
+gfx::Size SelectSrcCodedSize(TestConversionType conversion_type) {
+ switch (conversion_type) {
+ case TestConversionType::kNormal:
+ case TestConversionType::kScaled:
+ return kCodedSize;
+ case TestConversionType::kOdd:
+ return kOddRect.size();
+ }
+}
+
+gfx::Rect SelectSrcRect(TestConversionType conversion_type) {
+ switch (conversion_type) {
+ case TestConversionType::kNormal:
+ case TestConversionType::kScaled:
+ return kVisibleRect;
+ case TestConversionType::kOdd:
+ return kOddRect;
+ }
}
bool IsConversionSupported(VideoPixelFormat src, VideoPixelFormat dest) {
@@ -99,7 +139,8 @@
} // namespace
-using TestParams = testing::tuple<VideoPixelFormat, VideoPixelFormat, bool>;
+using TestParams =
+ testing::tuple<VideoPixelFormat, VideoPixelFormat, TestConversionType>;
class VideoFrameConverterTest
: public testing::Test,
public ::testing::WithParamInterface<TestParams> {
@@ -107,12 +148,16 @@
VideoFrameConverterTest()
: src_format_(testing::get<0>(GetParam())),
dest_format_(testing::get<1>(GetParam())),
+ src_coded_size_(SelectSrcCodedSize(testing::get<2>(GetParam()))),
+ src_visible_rect_(SelectSrcRect(testing::get<2>(GetParam()))),
dest_coded_size_(SelectDestSize(testing::get<2>(GetParam()))),
dest_visible_rect_(SelectDestRect(testing::get<2>(GetParam()))) {}
protected:
const VideoPixelFormat src_format_;
const VideoPixelFormat dest_format_;
+ const gfx::Size src_coded_size_;
+ const gfx::Rect src_visible_rect_;
const gfx::Size dest_coded_size_;
const gfx::Rect dest_visible_rect_;
VideoFrameConverter converter_;
@@ -121,7 +166,7 @@
TEST_P(VideoFrameConverterTest, ConvertAndScale) {
// Zero initialize so coded size regions are all zero.
auto src_frame = VideoFrame::CreateZeroInitializedFrame(
- src_format_, kCodedSize, kVisibleRect, kVisibleRect.size(),
+ src_format_, src_coded_size_, src_visible_rect_, src_visible_rect_.size(),
base::TimeDelta());
auto dest_frame = VideoFrame::CreateZeroInitializedFrame(
dest_format_, dest_coded_size_, dest_visible_rect_,
@@ -176,28 +221,47 @@
}
}
-INSTANTIATE_TEST_SUITE_P(,
- VideoFrameConverterTest,
- testing::Combine(testing::Values(PIXEL_FORMAT_XBGR,
- PIXEL_FORMAT_XRGB,
- PIXEL_FORMAT_ABGR,
- PIXEL_FORMAT_ARGB,
- PIXEL_FORMAT_I420,
- PIXEL_FORMAT_I420A,
- PIXEL_FORMAT_I444,
- PIXEL_FORMAT_I444A,
- PIXEL_FORMAT_NV12,
- PIXEL_FORMAT_NV12A),
- testing::Values(PIXEL_FORMAT_XBGR,
- PIXEL_FORMAT_XRGB,
- PIXEL_FORMAT_ABGR,
- PIXEL_FORMAT_ARGB,
- PIXEL_FORMAT_I420,
- PIXEL_FORMAT_I420A,
- PIXEL_FORMAT_I444,
- PIXEL_FORMAT_I444A,
- PIXEL_FORMAT_NV12,
- PIXEL_FORMAT_NV12A),
- testing::Bool()));
+TEST(VideoFrameConverterRegressionTest, WeirdScaling) {
+ constexpr gfx::Size kTestSize(80, 50);
+ auto src_frame = VideoFrame::CreateZeroInitializedFrame(
+ PIXEL_FORMAT_I420, kTestSize, gfx::Rect(kTestSize), kTestSize,
+ base::TimeDelta());
+ constexpr gfx::Size kDestSize(188, 144);
+ auto dest_frame = VideoFrame::CreateZeroInitializedFrame(
+ PIXEL_FORMAT_NV12, kDestSize, gfx::Rect(kDestSize), kDestSize,
+ base::TimeDelta());
+
+ FillFourColors(*src_frame);
+
+ VideoFrameConverter converter;
+ ASSERT_TRUE(converter.ConvertAndScale(*src_frame, *dest_frame).is_ok());
+}
+
+INSTANTIATE_TEST_SUITE_P(
+ ,
+ VideoFrameConverterTest,
+ testing::Combine(testing::Values(PIXEL_FORMAT_XBGR,
+ PIXEL_FORMAT_XRGB,
+ PIXEL_FORMAT_ABGR,
+ PIXEL_FORMAT_ARGB,
+ PIXEL_FORMAT_I420,
+ PIXEL_FORMAT_I420A,
+ PIXEL_FORMAT_I444,
+ PIXEL_FORMAT_I444A,
+ PIXEL_FORMAT_NV12,
+ PIXEL_FORMAT_NV12A),
+ testing::Values(PIXEL_FORMAT_XBGR,
+ PIXEL_FORMAT_XRGB,
+ PIXEL_FORMAT_ABGR,
+ PIXEL_FORMAT_ARGB,
+ PIXEL_FORMAT_I420,
+ PIXEL_FORMAT_I420A,
+ PIXEL_FORMAT_I444,
+ PIXEL_FORMAT_I444A,
+ PIXEL_FORMAT_NV12,
+ PIXEL_FORMAT_NV12A),
+ testing::Values(TestConversionType::kNormal,
+ TestConversionType::kScaled,
+ TestConversionType::kOdd)));
} // namespace media
diff --git a/media/base/video_frame_unittest.cc b/media/base/video_frame_unittest.cc
index cbc39743..5dcf7d20 100644
--- a/media/base/video_frame_unittest.cc
+++ b/media/base/video_frame_unittest.cc
@@ -797,9 +797,11 @@
if (plane == VideoFrame::Plane::kY || plane == VideoFrame::Plane::kA) {
EXPECT_LT(frame->GetVisibleRowBytes(plane), frame->row_bytes(plane));
EXPECT_LT(frame->GetVisibleRows(plane), frame->rows(plane));
+ EXPECT_LT(frame->GetVisibleColumns(plane), frame->columns(plane));
} else {
EXPECT_EQ(frame->GetVisibleRowBytes(plane), frame->row_bytes(plane));
EXPECT_EQ(frame->GetVisibleRows(plane), frame->rows(plane));
+ EXPECT_EQ(frame->GetVisibleColumns(plane), frame->columns(plane));
}
}
@@ -811,10 +813,11 @@
for (int plane = 0; plane < 4; plane++) {
EXPECT_EQ(frame->GetVisibleRowBytes(plane), frame->row_bytes(plane));
EXPECT_EQ(frame->GetVisibleRows(plane), frame->rows(plane));
+ EXPECT_EQ(frame->GetVisibleColumns(plane), frame->columns(plane));
}
}
-TEST(VideoFrame, RowBytes) {
+TEST(VideoFrame, RowsColumnsAndRowBytes) {
constexpr gfx::Size kCodedSize(16, 14);
constexpr gfx::Rect kVisibleRect(4, 4, 8, 8);
@@ -831,6 +834,10 @@
ASSERT_EQ(frame->rows(VideoFrame::Plane::kU), kCodedSize.height() / 2);
ASSERT_EQ(frame->rows(VideoFrame::Plane::kV), kCodedSize.height() / 2);
ASSERT_EQ(frame->rows(VideoFrame::Plane::kA), kCodedSize.height());
+ ASSERT_EQ(frame->columns(VideoFrame::Plane::kY), kCodedSize.width());
+ ASSERT_EQ(frame->columns(VideoFrame::Plane::kU), kCodedSize.width() / 2);
+ ASSERT_EQ(frame->columns(VideoFrame::Plane::kV), kCodedSize.width() / 2);
+ ASSERT_EQ(frame->columns(VideoFrame::Plane::kA), kCodedSize.width());
ASSERT_EQ(frame->GetVisibleRowBytes(VideoFrame::Plane::kY),
kVisibleRect.width());
@@ -848,6 +855,14 @@
kVisibleRect.height() / 2);
ASSERT_EQ(frame->GetVisibleRows(VideoFrame::Plane::kA),
kVisibleRect.height());
+ ASSERT_EQ(frame->GetVisibleColumns(VideoFrame::Plane::kY),
+ kVisibleRect.width());
+ ASSERT_EQ(frame->GetVisibleColumns(VideoFrame::Plane::kU),
+ kVisibleRect.width() / 2);
+ ASSERT_EQ(frame->GetVisibleColumns(VideoFrame::Plane::kV),
+ kVisibleRect.width() / 2);
+ ASSERT_EQ(frame->GetVisibleColumns(VideoFrame::Plane::kA),
+ kVisibleRect.width());
}
TEST(VideoFrame, AllocationSize_OddSize) {
Original Bug Report
Potential GPU Memory Leak via MergeUV Under-write on Odd Dimensions
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 without the security team.
Overview: A potential vulnerability in media::internals::MergeUV allows a compromised renderer to leak uninitialized GPU or Mediaserver memory. When converting video frames with odd dimensions to NV12, MergeUV uses floor division for UV plane size while allocations use ceil semantics, resulting in uninitialized edge bytes being encoded into the output.
Affected files:
media/base/video_frame_converter_internals.ccmedia/base/video_frame_converter.ccmedia/gpu/windows/mf_video_encoder_shared_state.ccmedia/gpu/android/ndk_video_encode_accelerator.cc
Estimated timestamp from git blame: 2024-05-13
Vulnerability Summary
A potential memory leak exists in media::internals::MergeUV. When converting frames with odd dimensions to NV12 format, the function under-writes the UV plane. The underlying uninitialized memory (from GPU or mediaserver heaps) is subsequently encoded by hardware video encoders and returned to the renderer, allowing an attacker to bypass ASLR or steal sensitive cross-process data.
Vulnerability Details
Initial logic and parameters are validated when a compromised renderer initializes a VideoEncodeAccelerator with odd dimensions (e.g., 1001x1001). Standard processing is applied to establish the Mojo IPC and hardware codec setup, leading directly to the VideoFrameConverter when scaling is required.
The core vulnerability is located in media/base/video_frame_converter_internals.cc. The NV12 format requires U and V planes to be subsampled by a factor of 2. For an odd dimension like 1001, the buffer allocation calculates the required size using ceil semantics (e.g., VideoFrame::Rows yields 501 rows). However, MergeUV uses floor division:
// From media/base/video_frame_converter_internals.cc
libyuv::MergeUVPlane(src_u, src_u_stride, src_v, src_v_stride,
dest_uv, dest_uv_stride,
dest.visible_rect().width() / 2, // Floor division
dest.visible_rect().height() / 2); // Floor division
For a 1001x1001 frame, this writes 500x500 chroma samples. The 501st row and the trailing columns remain unwritten. Because the backing buffers (such as those from Android’s AMediaCodec_getInputBuffer or Windows’ MFCreateAlignedMemoryBuffer) are typically uninitialized, these edge pixels retain stale data. The hardware encoder compresses the full 501x501 UV plane into the H.264/HEVC/VP9 bitstream, leaking the uninitialized memory to the renderer process.
Potential Attack Steps
Note: These are suggested steps; our tooling does not yet have the ability to run code.
- A compromised renderer initializes
media.mojom.VideoEncodeAcceleratorwith aConfigusing odd dimensions (e.g.,input_visible_size= {1001, 1001}). - Standard processing occurs to allocate the input buffers for the hardware encoder. These buffers contain uninitialized GPU/mediaserver heap memory.
- The renderer submits a
VideoFramewhere thevisible_rectslightly differs from thecoded_size, forcing the GPU process to route the frame throughVideoFrameConverter::ConvertAndScale. MergeUVinterleaves the U and V planes but stops one row/column short due to the floor division.- The hardware encoder reads the entire buffer, including the unwritten edge pixels, and encodes the uninitialized memory into the video bitstream.
- The compromised renderer decodes the returned bitstream and recovers the leaked bytes from the edge macroblocks.
Proposed Fix
Update MergeUV in media/base/video_frame_converter_internals.cc to use ceil semantics, aligning with the allocation logic in VideoFrame. For example:
(dest_frame.visible_rect().width() + 1) / 2,
(dest_frame.visible_rect().height() + 1) / 2);
Evaluated with Chrome root at commit: f200f57a19490707ff8bc7aa5de3cbc443a3afad
Results so far have been promising, but there can be wrong deductions. If this proves to be a false positive, please close as WAI; 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.