summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDale Curtis <dalecurtis@chromium.org>2023-03-16 08:11:22 +0000
committerMichael BrĂ¼ning <michael.bruning@qt.io>2023-04-03 15:25:26 +0000
commit76bad436d5eaf569f88b0e9adb6683c42645b786 (patch)
tree75e9a8c87f31f2674c378c8f160ec3afded8129b
parentf6a764fd4d5fa4eada45c3dea077af17583f40b4 (diff)
downloadqtwebengine-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.cc17
-rw-r--r--chromium/media/base/video_frame_layout.cc29
-rw-r--r--chromium/media/base/video_frame_layout.h7
-rw-r--r--chromium/media/mojo/mojom/video_frame_mojom_traits.cc10
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))