diff options
author | Dale Curtis <dalecurtis@chromium.org> | 2023-03-16 08:11:22 +0000 |
---|---|---|
committer | Michael BrĂ¼ning <michael.bruning@qt.io> | 2023-04-03 15:25:26 +0000 |
commit | 76bad436d5eaf569f88b0e9adb6683c42645b786 (patch) | |
tree | 75e9a8c87f31f2674c378c8f160ec3afded8129b | |
parent | f6a764fd4d5fa4eada45c3dea077af17583f40b4 (diff) | |
download | qtwebengine-chromium-76bad436d5eaf569f88b0e9adb6683c42645b786.tar.gz |
[Backport] CVE-2023-1532: Out of bounds read in GPU Video
Cherry-pick of patch originally reviewed on
https://chromium-review.googlesource.com/c/chromium/src/+/4342492:
Improve checks for VideoFrame layouts.
Adds a `VideoFrameLayout::IsValidForSize(size)` method which clients can
use to verify a VideoFrameLayout for their purpose. Updates a few call
sites to use the new verifier and adds tests.
(cherry picked from commit 17f73200c066158330542c19d521c517586533a2)
Fixed: 1421268
Change-Id: I51049ada6119eddb31cdd9b7edfe77ee65b1da7a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4307674
Auto-Submit: Dale Curtis <dalecurtis@chromium.org>
Commit-Queue: Dominick Ng <dominickn@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1116233}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4342492
Cr-Commit-Position: refs/branch-heads/5481@{#1361}
Cr-Branched-From: 130f3e4d850f4bc7387cfb8d08aa993d288a67a9-refs/heads/main@{#1084008}
(cherry picked from commit b5c9e5efe5dd5e3d19d3a4b7428ec99a85d5aa1f)
Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/468507
Reviewed-by: Michal Klocek <michal.klocek@qt.io>
-rw-r--r-- | chromium/media/base/video_frame.cc | 17 | ||||
-rw-r--r-- | chromium/media/base/video_frame_layout.cc | 29 | ||||
-rw-r--r-- | chromium/media/base/video_frame_layout.h | 7 | ||||
-rw-r--r-- | chromium/media/mojo/mojom/video_frame_mojom_traits.cc | 10 |
4 files changed, 46 insertions, 17 deletions
diff --git a/chromium/media/base/video_frame.cc b/chromium/media/base/video_frame.cc index 4aa077a5678..4ec81d5c1fc 100644 --- a/chromium/media/base/video_frame.cc +++ b/chromium/media/base/video_frame.cc @@ -427,20 +427,9 @@ scoped_refptr<VideoFrame> VideoFrame::WrapExternalDataWithLayout( StorageType storage_type = STORAGE_UNOWNED_MEMORY; if (!IsValidConfig(layout.format(), storage_type, layout.coded_size(), - visible_rect, natural_size)) { - DLOG(ERROR) << __func__ << " Invalid config." - << ConfigToString(layout.format(), storage_type, - layout.coded_size(), visible_rect, - natural_size); - return nullptr; - } - - const auto& last_plane = layout.planes()[layout.planes().size() - 1]; - const size_t required_size = last_plane.offset + last_plane.size; - if (data_size < required_size) { - DLOG(ERROR) << __func__ << " Provided data size is too small. Provided " - << data_size << " bytes, but " << required_size - << " bytes are required." + visible_rect, natural_size) || + !layout.FitsInContiguousBufferOfSize(data_size)) { + DLOG(ERROR) << "Invalid config: " << ConfigToString(layout.format(), storage_type, layout.coded_size(), visible_rect, natural_size); diff --git a/chromium/media/base/video_frame_layout.cc b/chromium/media/base/video_frame_layout.cc index b3307c302bf..1426adb193d 100644 --- a/chromium/media/base/video_frame_layout.cc +++ b/chromium/media/base/video_frame_layout.cc @@ -9,6 +9,7 @@ #include <sstream> #include "base/notreached.h" +#include "base/numerics/checked_math.h" namespace media { @@ -173,6 +174,34 @@ bool VideoFrameLayout::operator!=(const VideoFrameLayout& rhs) const { return !(*this == rhs); } +bool VideoFrameLayout::FitsInContiguousBufferOfSize(size_t data_size) const { + if (is_multi_planar_) { + return false; + } + + base::CheckedNumeric<size_t> required_size = 0; + for (const auto& plane : planes_) { + if (plane.offset > data_size || plane.size > data_size) { + return false; + } + + // No individual plane should have a size + offset > data_size. + base::CheckedNumeric<size_t> plane_end = plane.size; + plane_end += plane.offset; + if (!plane_end.IsValid() || plane_end.ValueOrDie() > data_size) { + return false; + } + + required_size += plane.size; + } + + if (!required_size.IsValid() || required_size.ValueOrDie() > data_size) { + return false; + } + + return true; +} + std::ostream& operator<<(std::ostream& ostream, const VideoFrameLayout& layout) { ostream << "VideoFrameLayout(format: " << layout.format() diff --git a/chromium/media/base/video_frame_layout.h b/chromium/media/base/video_frame_layout.h index d899c15d2d3..ae68bcfe5ab 100644 --- a/chromium/media/base/video_frame_layout.h +++ b/chromium/media/base/video_frame_layout.h @@ -112,6 +112,13 @@ class MEDIA_EXPORT VideoFrameLayout { // Return the modifier of buffers. uint64_t modifier() const { return modifier_; } + // Any constructible layout is valid in and of itself, it can only be invalid + // if the backing memory is too small to contain it. + // + // Returns true if this VideoFrameLayout can fit in a contiguous buffer of + // size `data_size` -- always false for multi-planar layouts. + bool FitsInContiguousBufferOfSize(size_t data_size) const; + private: VideoFrameLayout(VideoPixelFormat format, const gfx::Size& coded_size, diff --git a/chromium/media/mojo/mojom/video_frame_mojom_traits.cc b/chromium/media/mojo/mojom/video_frame_mojom_traits.cc index cdc4498f528..3a07ab5e7e8 100644 --- a/chromium/media/mojo/mojom/video_frame_mojom_traits.cc +++ b/chromium/media/mojo/mojom/video_frame_mojom_traits.cc @@ -231,14 +231,17 @@ bool StructTraits<media::mojom::VideoFrameDataView, auto layout = media::VideoFrameLayout::CreateWithPlanes(format, coded_size, std::move(planes)); - if (!layout) { + if (!layout || !layout->FitsInContiguousBufferOfSize(mapping.size())) { DLOG(ERROR) << "Invalid layout"; return false; } + frame = media::VideoFrame::WrapExternalYuvDataWithLayout( *layout, visible_rect, natural_size, addr[0], addr[1], addr[2], timestamp); - frame->BackWithOwnedSharedMemory(std::move(region), std::move(mapping)); + if (frame) { + frame->BackWithOwnedSharedMemory(std::move(region), std::move(mapping)); + } } else if (data.is_gpu_memory_buffer_data()) { media::mojom::GpuMemoryBufferVideoFrameDataDataView gpu_memory_buffer_data; data.GetGpuMemoryBufferDataDataView(&gpu_memory_buffer_data); @@ -313,8 +316,9 @@ bool StructTraits<media::mojom::VideoFrameDataView, return false; } - if (!frame) + if (!frame) { return false; + } media::VideoFrameMetadata metadata; if (!input.ReadMetadata(&metadata)) |