From 8b524801b75089fa6a6f5d8309e4e5ed4b0bba6c Mon Sep 17 00:00:00 2001 From: Michael Ludwig Date: Mon, 27 Jan 2020 20:44:51 +0000 Subject: [Backport] CVE-2020-6396 - Inappropriate implementation in Skia MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Manual backport of patch originally reviewed on https://chromium-review.googlesource.com/c/chromium/src/+/2023350: M80 merge: Preserve scissor for RPDQs with filters Cherry pick of https://chromium-review.googlesource.com/c/chromium/src/+/2019804 If the RPDQ has a filter, it's touched pixels are not actually restricted to the visible rect of the quad. In that case it is incorrect to explicitly clip the visible rect to the scissor and not set the scissor as a clipRect. This CL makes it so the scissor is remembered and is applied post-filtering, so effects like drop shadows are properly clipped to the window content. Bug: 1035271 Change-Id: Iaba086c2d6f679c659e99410a2ab3dffa7c7cc42 Reviewed-by: Jüri Valdmann --- .../viz/service/display/skia_renderer.cc | 55 +++++++++++++--------- .../components/viz/service/display/skia_renderer.h | 7 +++ 2 files changed, 39 insertions(+), 23 deletions(-) diff --git a/chromium/components/viz/service/display/skia_renderer.cc b/chromium/components/viz/service/display/skia_renderer.cc index 448d20b1eca..3b9c87435d5 100644 --- a/chromium/components/viz/service/display/skia_renderer.cc +++ b/chromium/components/viz/service/display/skia_renderer.cc @@ -98,29 +98,6 @@ bool IsTextureResource(DisplayResourceProvider* resource_provider, return !resource_provider->IsResourceSoftwareBacked(resource_id); } -bool CanExplicitlyScissor(const DrawQuad* quad, - const gfx::QuadF* draw_region, - const gfx::Transform& contents_device_transform) { - // PICTURE_CONTENT is not like the others, since it is executing a list of - // draw calls into the canvas. - if (quad->material == DrawQuad::Material::kPictureContent) - return false; - // Intersection with scissor and a quadrilateral is not necessarily a quad, - // so don't complicate things - if (draw_region) - return false; - - // This is slightly different than - // gfx::Transform::IsPositiveScaleAndTranslation in that it also allows zero - // scales. This is because in the common orthographic case the z scale is 0. - if (!contents_device_transform.IsScaleOrTranslation()) - return false; - - return contents_device_transform.matrix().get(0, 0) >= 0.0 && - contents_device_transform.matrix().get(1, 1) >= 0.0 && - contents_device_transform.matrix().get(2, 2) >= 0.0; -} - void ApplyExplicitScissor(const DrawQuad* quad, const gfx::Rect& scissor_rect, const gfx::Transform& device_transform, @@ -1014,6 +991,38 @@ SkiaRenderer::DrawQuadParams SkiaRenderer::CalculateDrawQuadParams( return params; } +bool SkiaRenderer::CanExplicitlyScissor( + const DrawQuad* quad, + const gfx::QuadF* draw_region, + const gfx::Transform& contents_device_transform) const { + // PICTURE_CONTENT is not like the others, since it is executing a list of + // draw calls into the canvas. + if (quad->material == DrawQuad::Material::kPictureContent) + return false; + // Intersection with scissor and a quadrilateral is not necessarily a quad, + // so don't complicate things + if (draw_region) + return false; + + // This is slightly different than + // gfx::Transform::IsPositiveScaleAndTranslation in that it also allows zero + // scales. This is because in the common orthographic case the z scale is 0. + if (!contents_device_transform.IsScaleOrTranslation()) + return false; + + if (quad->material == DrawQuad::Material::kRenderPass) { + // If the renderpass has filters, the filters may modify the effective + // geometry beyond the quad's visible_rect, so it's not safe to pre-clip. + auto pass_id = RenderPassDrawQuad::MaterialCast(quad)->render_pass_id; + if (FiltersForPass(pass_id) || BackdropFiltersForPass(pass_id)) + return false; + } + + return contents_device_transform.matrix().get(0, 0) >= 0.0 && + contents_device_transform.matrix().get(1, 1) >= 0.0 && + contents_device_transform.matrix().get(2, 2) >= 0.0; +} + SkCanvas::ImageSetEntry SkiaRenderer::MakeEntry(const SkImage* image, int matrix_index, const DrawQuadParams& params) { diff --git a/chromium/components/viz/service/display/skia_renderer.h b/chromium/components/viz/service/display/skia_renderer.h index f7134596734..8d86c0630a9 100644 --- a/chromium/components/viz/service/display/skia_renderer.h +++ b/chromium/components/viz/service/display/skia_renderer.h @@ -123,6 +123,13 @@ class VIZ_SERVICE_EXPORT SkiaRenderer : public DirectRenderer { const gfx::RectF& valid_texel_bounds, DrawQuadParams* params); + // True or false if the DrawQuad can have the scissor rect applied by + // modifying the quad's visible_rect instead of as a separate clip operation. + bool CanExplicitlyScissor( + const DrawQuad* quad, + const gfx::QuadF* draw_region, + const gfx::Transform& contents_device_transform) const; + bool MustFlushBatchedQuads(const DrawQuad* new_quad, const DrawQuadParams& params); void AddQuadToBatch(const SkImage* image, -- cgit v1.2.1