mirror of
https://github.com/electron/electron.git
synced 2026-05-02 03:00:22 -04:00
fix: validate OSR frame geometry against shared-memory mapping size (#50940)
Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com> Co-authored-by: Sam Attard <sattard@anthropic.com>
This commit is contained in:
@@ -4,6 +4,7 @@
|
||||
|
||||
#include "shell/browser/osr/osr_host_display_client.h"
|
||||
|
||||
#include <optional>
|
||||
#include <utility>
|
||||
|
||||
#include "components/viz/common/resources/shared_image_format_utils.h"
|
||||
@@ -38,26 +39,33 @@ void LayeredWindowUpdater::OnAllocatedSharedMemory(
|
||||
if (!region.IsValid())
|
||||
return;
|
||||
|
||||
// Make sure |pixel_size| is sane.
|
||||
if (!SharedMemorySizeForSharedImageFormat(viz::SinglePlaneFormat::kRGBA_8888,
|
||||
pixel_size)) {
|
||||
// |pixel_size| and |region| arrive from the GPU process. Reject geometry that
|
||||
// overflows or that claims more pixels than the region can actually back so a
|
||||
// hostile peer cannot make the canvas read past the shared-memory mapping.
|
||||
std::optional<size_t> expected_bytes = SharedMemorySizeForSharedImageFormat(
|
||||
viz::SinglePlaneFormat::kRGBA_8888, pixel_size);
|
||||
if (!expected_bytes || region.GetSize() < *expected_bytes) {
|
||||
DLOG(ERROR) << "Shared memory region too small for pixel_size "
|
||||
<< pixel_size.ToString();
|
||||
return;
|
||||
}
|
||||
|
||||
#if defined(WIN32)
|
||||
canvas_ = skia::CreatePlatformCanvasWithSharedSection(
|
||||
pixel_size.width(), pixel_size.height(), false,
|
||||
region.GetPlatformHandle(), skia::CRASH_ON_FAILURE);
|
||||
region.GetPlatformHandle(), skia::RETURN_NULL_ON_FAILURE);
|
||||
#else
|
||||
shm_mapping_ = region.Map();
|
||||
if (!shm_mapping_.IsValid()) {
|
||||
if (!shm_mapping_.IsValid() || shm_mapping_.size() < *expected_bytes) {
|
||||
DLOG(ERROR) << "Failed to map shared memory region";
|
||||
shm_mapping_ = base::WritableSharedMemoryMapping();
|
||||
return;
|
||||
}
|
||||
|
||||
canvas_ = skia::CreatePlatformCanvasWithPixels(
|
||||
pixel_size.width(), pixel_size.height(), false,
|
||||
static_cast<uint8_t*>(shm_mapping_.memory()), 0, skia::CRASH_ON_FAILURE);
|
||||
static_cast<uint8_t*>(shm_mapping_.memory()), 0,
|
||||
skia::RETURN_NULL_ON_FAILURE);
|
||||
#endif
|
||||
}
|
||||
|
||||
@@ -66,7 +74,7 @@ void LayeredWindowUpdater::Draw(const gfx::Rect& damage_rect,
|
||||
SkPixmap pixmap;
|
||||
SkBitmap bitmap;
|
||||
|
||||
if (active_ && canvas_->peekPixels(&pixmap)) {
|
||||
if (active_ && canvas_ && canvas_->peekPixels(&pixmap)) {
|
||||
bitmap.installPixels(pixmap);
|
||||
callback_.Run(damage_rect, bitmap, {});
|
||||
}
|
||||
|
||||
@@ -14,6 +14,7 @@
|
||||
#include "shell/browser/osr/osr_render_widget_host_view.h"
|
||||
#include "third_party/skia/include/core/SkImageInfo.h"
|
||||
#include "third_party/skia/include/core/SkRegion.h"
|
||||
#include "ui/gfx/geometry/rect.h"
|
||||
#include "ui/gfx/skbitmap_operations.h"
|
||||
|
||||
namespace {
|
||||
@@ -167,6 +168,27 @@ void OffScreenVideoConsumer::OnFrameCaptured(
|
||||
return;
|
||||
}
|
||||
|
||||
// |content_rect| and |info->coded_size| arrive from the GPU process over
|
||||
// Mojo. The bitmap below is sized from |content_rect| but backed by the
|
||||
// shared-memory mapping, so a hostile peer could send a tiny region with an
|
||||
// oversized |content_rect| and have us read past the mapping. Clamp both the
|
||||
// geometry and the resulting byte size to the mapping before wrapping it.
|
||||
const size_t row_bytes =
|
||||
media::VideoFrame::RowBytes(media::VideoFrame::Plane::kARGB,
|
||||
info->pixel_format, info->coded_size.width());
|
||||
const SkImageInfo image_info = SkImageInfo::MakeN32(
|
||||
content_rect.width(), content_rect.height(), kPremul_SkAlphaType);
|
||||
const size_t required_bytes = image_info.computeByteSize(row_bytes);
|
||||
if (!gfx::Rect(info->coded_size).Contains(content_rect) ||
|
||||
SkImageInfo::ByteSizeOverflowed(required_bytes) ||
|
||||
required_bytes > mapping.size()) {
|
||||
DLOG(ERROR) << "content_rect " << content_rect.ToString()
|
||||
<< " is not bounded by coded_size "
|
||||
<< info->coded_size.ToString() << " / mapping size "
|
||||
<< mapping.size();
|
||||
return;
|
||||
}
|
||||
|
||||
// The SkBitmap's pixels will be marked as immutable, but the installPixels()
|
||||
// API requires a non-const pointer. So, cast away the const.
|
||||
void* const pixels = const_cast<void*>(mapping.memory());
|
||||
@@ -185,11 +207,7 @@ void OffScreenVideoConsumer::OnFrameCaptured(
|
||||
|
||||
SkBitmap bitmap;
|
||||
bitmap.installPixels(
|
||||
SkImageInfo::MakeN32(content_rect.width(), content_rect.height(),
|
||||
kPremul_SkAlphaType),
|
||||
pixels,
|
||||
media::VideoFrame::RowBytes(media::VideoFrame::Plane::kARGB,
|
||||
info->pixel_format, info->coded_size.width()),
|
||||
image_info, pixels, row_bytes,
|
||||
[](void* addr, void* context) {
|
||||
delete static_cast<FramePinner*>(context);
|
||||
},
|
||||
|
||||
Reference in New Issue
Block a user