Skip to content

Commit

Permalink
Use a resource after Free in OffscreenCanvasRC::DrawTextInternal()
Browse files Browse the repository at this point in the history
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 15c4ec7)

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: 740e9e8-refs/heads/master@{#782793}
  • Loading branch information
yiyix authored and Commit Bot committed Sep 1, 2020
1 parent 1b78188 commit 393e2c2
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 2 deletions.
Expand Up @@ -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(
Expand Down
@@ -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>

0 comments on commit 393e2c2

Please sign in to comment.