From 393e2c2632da6436b1d5d8bdb52e14654fc004c2 Mon Sep 17 00:00:00 2001
From: yiyix <yiyix@chromium.org>
Date: Tue, 1 Sep 2020 20:29:42 +0000
Subject: [PATCH] Use a resource after Free in
 OffscreenCanvasRC::DrawTextInternal()

In OffscreenCanvasRenderingContext::DrawTextInternal(), |paint_canvas|
can be freed in the draw command in BaseRenderingContext. We then use
the |paint_canvas| causes the security bug that we are using a resource
after it's freed.

Looking at how |paint_canvas| is used in the method DrawTextInternal(),
restore a cleared |paint_canvas| is not really necessary. So I removed
it's only restored if the canvas is not cleared (i.e. canvas is not
freed).

Bug: 1111737

TBR=fserb@chromium.org

(cherry picked from commit 15c4ec7c0cc6197c5b0cbaf0d05d6fb9a667e358)

Change-Id: I699b855434f7ddfbc678d2a9cfe25fe4938a798a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2358574
Commit-Queue: Yi Xu <yiyix@chromium.org>
Reviewed-by: Fernando Serboncini <fserb@chromium.org>
Reviewed-by: Aaron Krajeski <aaronhk@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#802508}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2388264
Reviewed-by: Yi Xu <yiyix@chromium.org>
Cr-Commit-Position: refs/branch-heads/4183@{#1732}
Cr-Branched-From: 740e9e8a40505392ba5c8e022a8024b3d018ca65-refs/heads/master@{#782793}
---
 .../offscreen_canvas_rendering_context_2d.cc  | 10 ++++++--
 .../fast/canvas/OffscreenCanvas-drawText.html | 23 +++++++++++++++++++
 2 files changed, 31 insertions(+), 2 deletions(-)
 create mode 100644 third_party/blink/web_tests/fast/canvas/OffscreenCanvas-drawText.html

diff --git a/third_party/blink/renderer/modules/canvas/offscreencanvas2d/offscreen_canvas_rendering_context_2d.cc b/third_party/blink/renderer/modules/canvas/offscreencanvas2d/offscreen_canvas_rendering_context_2d.cc
index abbbe9a8aa215..24460a75f4308 100644
--- a/third_party/blink/renderer/modules/canvas/offscreencanvas2d/offscreen_canvas_rendering_context_2d.cc
+++ b/third_party/blink/renderer/modules/canvas/offscreencanvas2d/offscreen_canvas_rendering_context_2d.cc
@@ -583,8 +583,14 @@ void OffscreenCanvasRenderingContext2D::DrawTextInternal(
       [](const SkIRect& rect)  // overdraw test lambda
       { return false; },
       bounds, paint_type, CanvasRenderingContext2DState::kNoImage);
-  paint_canvas->restoreToCount(save_count);
-  ValidateStateStack();
+
+  // |paint_canvas| maybe rese during Draw. If that happens,
+  // GetOrCreatePaintCanvas will create a new |paint_canvas| and return a new
+  // address. In this case, there is no need to call |restoreToCount|.
+  if (paint_canvas == GetOrCreatePaintCanvas()) {
+    paint_canvas->restoreToCount(save_count);
+    ValidateStateStack();
+  }
 }
 
 TextMetrics* OffscreenCanvasRenderingContext2D::measureText(
diff --git a/third_party/blink/web_tests/fast/canvas/OffscreenCanvas-drawText.html b/third_party/blink/web_tests/fast/canvas/OffscreenCanvas-drawText.html
new file mode 100644
index 0000000000000..6a2e1f8d44dd9
--- /dev/null
+++ b/third_party/blink/web_tests/fast/canvas/OffscreenCanvas-drawText.html
@@ -0,0 +1,23 @@
+<script src="../../resources/testharness.js"></script>
+<script src="../../resources/testharnessreport.js"></script>
+<script type="text/javasctipt" id="worker">
+  var offscreenCanvas = new OffscreenCanvas(100, 100);
+  var ctx = offscreenCanvas.getContext("2d");
+  ctx.globalCompositeOperation = "copy";
+  ctx.rect(10, 10, 150, 100);
+  ctx.fill("evenodd");
+  ctx.lineTo(1,1);
+  ctx.fillText("", 1, 1);
+</script>
+<script>
+test(function() {
+    const worker = new Worker(
+        URL.createObjectURL(
+            new Blob(
+                [document.querySelector("#worker").textContent],
+                {type: 'text/javascript'}
+            )
+        )
+    );
+}, "crbug.com/1111737, pass by not crashing.");
+</script>