diff --git a/remoting/codec/BUILD.gn b/remoting/codec/BUILD.gn index 75608885a3079..306575f3f1192 100644 --- a/remoting/codec/BUILD.gn +++ b/remoting/codec/BUILD.gn @@ -87,7 +87,10 @@ if (!is_component_build && (is_win || is_linux)) { source_set("unit_tests") { testonly = true - sources = [ "encoder_bitrate_filter_unittest.cc" ] + sources = [ + "desktop_frame_rotation_oob_test.cc", + "encoder_bitrate_filter_unittest.cc", + ] deps = [ ":encoder", diff --git a/remoting/codec/desktop_frame_rotation_oob_test.cc b/remoting/codec/desktop_frame_rotation_oob_test.cc new file mode 100644 index 0000000000000..fc4717c88d048 --- /dev/null +++ b/remoting/codec/desktop_frame_rotation_oob_test.cc @@ -0,0 +1,122 @@ +// Copyright 2024 The Chromium Authors +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. +// +// Demonstrates that webrtc::RotateDesktopFrame() performs a heap +// out-of-bounds write when the live source-frame size diverges from the +// cached size used by the caller to derive |source_rect| -- the exact +// scenario that DxgiOutputDuplicator::Duplicate() can produce on Windows +// when IDXGIOutputDuplication::AcquireNextFrame() delivers a texture whose +// dimensions differ from the dimensions cached at Initialize() time. +// +// The bounds checks in RotateDesktopFrame() at desktop_frame_rotation.cc:102 +// and :108 are RTC_DCHECK-only and therefore compiled out in release builds +// (dcheck_always_on=false), so the function proceeds to call +// libyuv::ARGBRotate() with a destination pointer that lies outside the +// target heap allocation. +// +// This test mirrors the data-flow in dxgi_output_duplicator.cc:231-240: +// source_rect = RotateRect(updated_rect, cached_desktop_size, +// ReverseRotation(rotation)); +// RotateDesktopFrame(source /* live size */, source_rect, rotation, +// offset, target /* sized to cached_desktop_size */); + +#include "testing/gtest/include/gtest/gtest.h" +#include "third_party/libyuv/include/libyuv/cpu_id.h" +#include "third_party/webrtc/modules/desktop_capture/desktop_frame.h" +#include "third_party/webrtc/modules/desktop_capture/desktop_frame_rotation.h" +#include "third_party/webrtc/modules/desktop_capture/desktop_geometry.h" + +namespace { + +// libyuv::ARGBRotate dispatches to hand-written SSE2/AVX assembly stores +// which AddressSanitizer cannot instrument, so on a SIMD path the OOB write +// silently corrupts adjacent heap (observed as a later bad-free). Forcing +// the portable C row functions makes ASan report the precise OOB store. The +// out-of-bounds write occurs identically on the SIMD path; this only changes +// observability, not the bug. +struct ForceLibyuvCPath { + ForceLibyuvCPath() { libyuv::MaskCpuFlags(libyuv::kCpuInitialized); } + ~ForceLibyuvCPath() { libyuv::MaskCpuFlags(0); } +}; + +// Case 1: live texture height GREATER than cached -> heap-buffer-overflow +// WRITE past the end of |target|. +TEST(RotateDesktopFrameOOB, LiveTextureLargerThanCached_OverflowWrite) { + ForceLibyuvCPath force_c; + constexpr webrtc::Rotation kRotation = webrtc::Rotation::CLOCK_WISE_90; + + // Cached state captured once at DxgiOutputDuplicator construction / + // Initialize() (dxgi_output_duplicator.cc:79,147). + const webrtc::DesktopSize cached_desktop_size(100, 200); // post-rotation + const webrtc::DesktopSize cached_unrotated_size(200, 100); // pre-rotation + + // Live texture dimensions read fresh from texture->GetDesc() on this frame + // (dxgi_texture.cc:61-63). Height grew 100 -> 150. + const webrtc::DesktopSize live_texture_size(200, 150); + + // updated_region is clamped to cached desktop_size at + // dxgi_output_duplicator.cc:302; full-screen dirty rect maximises extent. + const webrtc::DesktopRect updated_rect = + webrtc::DesktopRect::MakeSize(cached_desktop_size); + + // dxgi_output_duplicator.cc:237-238 + const webrtc::DesktopRect source_rect = webrtc::RotateRect( + updated_rect, cached_desktop_size, webrtc::ReverseRotation(kRotation)); + ASSERT_TRUE(source_rect.equals( + webrtc::DesktopRect::MakeSize(cached_unrotated_size))); + + // |source| wraps the live DXGI surface (dxgi_texture.cc:68-72). + webrtc::BasicDesktopFrame source(live_texture_size); + // |target| is a heap-allocated BasicDesktopFrame sized to the cached + // desktop_size (dxgi_frame.cc:61, desktop_frame.cc:187-192). + webrtc::BasicDesktopFrame target(cached_desktop_size); + + // Passes the L197 guard: target.size() == cached_desktop_size contains + // GetTranslatedDesktopRect(offset=0,0) == cached_desktop_size. + // + // Inside RotateDesktopFrame: + // target_rect = RotateRect(source_rect=[0,0,200,100], + // source.size()=200x150, CW_90) + // = MakeXYWH(150-100, 0, 100, 200) = [50,0,150,200] + // target.size()=100x200 does NOT contain [50,0,150,200] (right=150>100) + // -> RTC_DCHECK at L108 would fire, but is compiled out. + // GetFrameDataAtPos((50,0)) = data + 200; ARGBRotate then writes + // 100*4 bytes per row for 200 rows with stride 400, so the last output + // row writes data[79800..80200) -- 200 bytes past the 80000-byte + // allocation. ASAN: heap-buffer-overflow WRITE. + webrtc::RotateDesktopFrame(source, source_rect, kRotation, + webrtc::DesktopVector(), &target); +} + +// Case 2: live texture height SMALLER than cached -> negative target_rect.x +// -> heap underflow WRITE before |target->data()|. +TEST(RotateDesktopFrameOOB, LiveTextureSmallerThanCached_UnderflowWrite) { + ForceLibyuvCPath force_c; + constexpr webrtc::Rotation kRotation = webrtc::Rotation::CLOCK_WISE_90; + + const webrtc::DesktopSize cached_desktop_size(100, 200); + const webrtc::DesktopSize cached_unrotated_size(200, 100); + // Height shrank 100 -> 60. + const webrtc::DesktopSize live_texture_size(200, 60); + + const webrtc::DesktopRect updated_rect = + webrtc::DesktopRect::MakeSize(cached_desktop_size); + const webrtc::DesktopRect source_rect = webrtc::RotateRect( + updated_rect, cached_desktop_size, webrtc::ReverseRotation(kRotation)); + ASSERT_TRUE(source_rect.equals( + webrtc::DesktopRect::MakeSize(cached_unrotated_size))); + + webrtc::BasicDesktopFrame source(live_texture_size); + webrtc::BasicDesktopFrame target(cached_desktop_size); + + // target_rect = RotateRect([0,0,200,100], 200x60, CW_90) + // = MakeXYWH(60-100, 0, 100, 200) = [-40, 0, 60, 200] + // GetFrameDataAtPos((-40,0)) = data - 160 -> underflow write. + // (source_rect also exceeds source.size(), giving an OOB read; ASAN + // reports whichever it touches first.) + webrtc::RotateDesktopFrame(source, source_rect, kRotation, + webrtc::DesktopVector(), &target); +} + +} // namespace