Tainted canvases security issue breaking many extensions (latest DEV builds)
Reported by
collin.c...@blueprairie.com,
Dec 14
|
|||||||||||
Issue descriptionUserAgent: Mozilla/5.0 (Windows NT 6.1; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/73.0.3639.1 Safari/537.36 Steps to reproduce the problem: 1. Install/use many various extensions utilizing favicons and htmlcanvas calls 2. Security-related errors prevent previously working extension builds from even initializing What is the expected behavior? Working extensions What went wrong? Several very popular extensions (I am still testing and coming across more and more) have been rendered once again totally broken via new (apparently automated) Chrome DEV builds that I have been told due to "automated builds" are not fully tested. ---------------------------------------------------------------------- One example: ---------------------------------------------------------------------- Extension: TheGreatSuspender URL: https://chrome.google.com/webstore/detail/the-great-suspender/klbibkeccnjlkjkiokjodocebajanakg Users: 1.5 MILLION users Current errors with last two DEV builds: When calling: const imageData = context.getImageData(... Terminating Error: Uncaught (in promise) SecurityError: Failed to execute 'getImageData' on 'CanvasRenderingContext2D': The canvas has been tainted by cross-origin data. Outcome: Extension initializes partially unable to function suspends due to this security error ---------------------------------------------------------------------- ---------------------------------------------------------------------- Another example: ---------------------------------------------------------------------- Extension: Bookmark Favicon Changer URL: https://chrome.google.com/webstore/detail/bookmark-favicon-changer/acmfnomgphggonodopogfbmkneepfgnh Users: 36,708 users Current errors with last two DEV builds: When calling: var imgDataURL = canvas.toDataURL("image/png", ""); Terminating Error: Uncaught SecurityError: Failed to execute 'toDataURL' on 'HTMLCanvasElement': Tainted canvases may not be exported. Outcome: Extension initializes partially unable to function due to this security error ---------------------------------------------------------------------- Did this work before? Yes 72.0.3622 (Unable to re-verify but believe this was last unaffected version) Chrome version: 73.0.3639.1 Channel: dev OS Version: 6.1 (Windows 7, Windows Server 2008 R2) Flash Version: As you can see there is a clear pattern where again most likely another revert gone bad related to canvas taint security prevention is causing catastrophic failures making it impossible to use many previously working extensions. If I roll back the Chrome DEV build to a fairly older one that I have used frequently for testing other similar revert issues (version 71.0.3578.20 DEV x64), these issues are gone and the current extension builds (including their latest Github DEV builds) all return to normal operation so it is clearly a recently introduced Chromium issue. I believe 72.0.3622 was the last unaffected version not displaying these issues, but I do not have that version readily available for testing ATM so cannot retest/reverify.
,
Dec 15
,
Dec 16
Bisected to r611093 = 04dc53eaaac123d2dff125aba572d4195d095725 = https://crrev.com/c/1275469 by yhirano@chromium.org "Remove the "origin" parameter from ImageResource::IsAccessAllowed" Landed in 72.0.3624.0 Simplified repro: 1. install Bookmark Favicon Changer https://chrome.google.com/webstore/detail/acmfnomgphggonodopogfbmkneepfgnh 2. open https://www.example.org 3. click the extension icon Expected: a big black popup opens (see the screenshot) Observed: a standard small context menu is shown
,
Dec 16
A popular Google extension is now completely broken: > Search by Image (by Google) (https://chrome.google.com/webstore/detail/search-by-image-by-google/dajedkncpodkggklbegccjpmnglmnflm)
,
Dec 18
Able to reproduce the issue on reported version 73.0.3639.1 and on Beta# 72.0.3626.17 the same is not seen on latest canary 73.0.3643.0 and stable# 71.0.3578.98 Windows 10, Ubuntu 14.04 & Mac 10.12.6, hence providing reverse bisect info Reverse Bisect Info: ================ Last Bad build: 73.0.3639.1 First Good build: 73.0.3640.0 You are probably looking for a change made after 616245 (known good), but no later than 616246 (first known bad). https://chromium.googlesource.com/chromium/src/+log/a4422dd1930e3d7fd32d05768104197eb0d1fd32..ff9707c095926abd5470537a146abcddb979e192 Change-Id: Iff56e3fc08f14b163d8fef425caeca42e1b6f3e0 Reviewed-on: https://chromium-review.googlesource.com/c/1375297 @Matt Falkenhagen: Please confirm the issue and help in re-assigning if it is not related to your change, please help in merging it to M-72 if applicable Adding ReleaseBlock-Stable for M-72, feel free to remove it if not applicable. Thanks!
,
Dec 18
Thanks looking. viswa.karala: which extension did you use to reproduce for the bisect? I'm curious why comment #3 came up with a different bisect result.
,
Dec 18
,
Dec 18
Re to C# 6: Reproduced the issue with extension and steps provided in comment# 3. Thanks!
,
Dec 18
#3 is for the breakage, #5 is for the fix, AFAICT.
,
Dec 18
#9: Ahhhhh, I totally missed that! Thanks.
,
Dec 18
yhirano: My CL doesn't merge cleanly to 72, because https://chromium-review.googlesource.com/c/1362774 also landed in 73 (at least, maybe others). Seems like we could: 1) Manually merge https://chromium-review.googlesource.com/c/1375297 to 72 without the origin taint change. 2) Merge https://chromium-review.googlesource.com/c/1362774 and https://chromium-review.googlesource.com/c/1375297 both to 72. 3) Revert https://crrev.com/c/1275469 from 72. I'm thinking 3) could be the safest option, since it landed just a few days before the branch cut. Thoughts?
,
Dec 18
Can we cleanly revert https://crrev.com/c/1275469 in 72?
,
Dec 18
I'm investigating but I suspect so... it landed on Nov 27 and we branched on Nov 29.
,
Dec 18
Then 3) seems the safest.
,
Dec 18
I've verified that reverting https://crrev.com/c/1275469 on 72 makes the extension from comment 3 work again. Requesting approval to revert this change from 72, i.e., approval to submit https://chromium-review.googlesource.com/c/chromium/src/+/1381398.
,
Dec 18
This bug requires manual review: M72 has already been promoted to the beta branch, so this requires manual review Please contact the milestone owner if you have questions. Owners: govind@(Android), kariahda@(iOS), djmm@(ChromeOS), abdulsyed@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Dec 18
Are we confident this won't break anything additionally? Since this CL had landed on Nov 27th, are we confident this will be a safe revert on 72?
,
Dec 18
Yes, the CL landed on Nov 27 and wasn't expected to have any behavior change, and M72 branched on Nov 29. It could be not safe if a change landed between Nov 27 and Nov 29 or was merged on later that built on top of it, which is unlikely since it wasn't expected to change behavior. I've verified reverting it on M72 builds cleanly and fixes the extension in comment 3. It'd provide additional confidence if all tests pass but there's no CQ for that as I understand it. I can merge and watch the beta builders.
,
Dec 18
Thanks - approving merge for M72. branch:3626
,
Dec 18
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/cb4cf0bf96ffe158db13a5066cb2670560a86720 commit cb4cf0bf96ffe158db13a5066cb2670560a86720 Author: Matt Falkenhagen <falken@chromium.org> Date: Tue Dec 18 23:23:57 2018 M72: Revert "Remove the "origin" parameter from ImageResource::IsAccessAllowed" This reverts commit 04dc53eaaac123d2dff125aba572d4195d095725. Reason: The change broke some extensions. Bug: 915384 Change-Id: Idae9fba283cfb5a904a6bc96c73cfb94cd1d64b2 Reviewed-on: https://chromium-review.googlesource.com/c/1381398 Reviewed-by: Matt Falkenhagen <falken@chromium.org> Reviewed-by: Yutaka Hirano <yhirano@chromium.org> Cr-Commit-Position: refs/branch-heads/3626@{#457} Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437} [modify] https://crrev.com/cb4cf0bf96ffe158db13a5066cb2670560a86720/third_party/blink/renderer/core/css/cssom/css_style_image_value.h [modify] https://crrev.com/cb4cf0bf96ffe158db13a5066cb2670560a86720/third_party/blink/renderer/core/html/canvas/canvas_image_source.h [modify] https://crrev.com/cb4cf0bf96ffe158db13a5066cb2670560a86720/third_party/blink/renderer/core/html/canvas/canvas_rendering_context.cc [modify] https://crrev.com/cb4cf0bf96ffe158db13a5066cb2670560a86720/third_party/blink/renderer/core/html/canvas/canvas_rendering_context.h [modify] https://crrev.com/cb4cf0bf96ffe158db13a5066cb2670560a86720/third_party/blink/renderer/core/html/canvas/html_canvas_element.cc [modify] https://crrev.com/cb4cf0bf96ffe158db13a5066cb2670560a86720/third_party/blink/renderer/core/html/canvas/html_canvas_element.h [modify] https://crrev.com/cb4cf0bf96ffe158db13a5066cb2670560a86720/third_party/blink/renderer/core/html/canvas/image_element_base.cc [modify] https://crrev.com/cb4cf0bf96ffe158db13a5066cb2670560a86720/third_party/blink/renderer/core/html/canvas/image_element_base.h [modify] https://crrev.com/cb4cf0bf96ffe158db13a5066cb2670560a86720/third_party/blink/renderer/core/html/media/html_video_element.cc [modify] https://crrev.com/cb4cf0bf96ffe158db13a5066cb2670560a86720/third_party/blink/renderer/core/html/media/html_video_element.h [modify] https://crrev.com/cb4cf0bf96ffe158db13a5066cb2670560a86720/third_party/blink/renderer/core/imagebitmap/image_bitmap.cc [modify] https://crrev.com/cb4cf0bf96ffe158db13a5066cb2670560a86720/third_party/blink/renderer/core/imagebitmap/image_bitmap.h [modify] https://crrev.com/cb4cf0bf96ffe158db13a5066cb2670560a86720/third_party/blink/renderer/core/layout/shapes/shape_outside_info.cc [modify] https://crrev.com/cb4cf0bf96ffe158db13a5066cb2670560a86720/third_party/blink/renderer/core/loader/resource/image_resource.cc [modify] https://crrev.com/cb4cf0bf96ffe158db13a5066cb2670560a86720/third_party/blink/renderer/core/loader/resource/image_resource.h [modify] https://crrev.com/cb4cf0bf96ffe158db13a5066cb2670560a86720/third_party/blink/renderer/core/loader/resource/image_resource_content.cc [modify] https://crrev.com/cb4cf0bf96ffe158db13a5066cb2670560a86720/third_party/blink/renderer/core/loader/resource/image_resource_content.h [modify] https://crrev.com/cb4cf0bf96ffe158db13a5066cb2670560a86720/third_party/blink/renderer/core/loader/resource/image_resource_info.h [modify] https://crrev.com/cb4cf0bf96ffe158db13a5066cb2670560a86720/third_party/blink/renderer/core/offscreencanvas/offscreen_canvas.h [modify] https://crrev.com/cb4cf0bf96ffe158db13a5066cb2670560a86720/third_party/blink/renderer/core/svg/svg_fe_image_element.cc [modify] https://crrev.com/cb4cf0bf96ffe158db13a5066cb2670560a86720/third_party/blink/renderer/modules/canvas/canvas2d/base_rendering_context_2d.cc [modify] https://crrev.com/cb4cf0bf96ffe158db13a5066cb2670560a86720/third_party/blink/renderer/modules/canvas/canvas2d/base_rendering_context_2d.h [modify] https://crrev.com/cb4cf0bf96ffe158db13a5066cb2670560a86720/third_party/blink/renderer/modules/canvas/canvas2d/canvas_rendering_context_2d.h [modify] https://crrev.com/cb4cf0bf96ffe158db13a5066cb2670560a86720/third_party/blink/renderer/modules/canvas/canvas2d/canvas_rendering_context_2d_test.cc [modify] https://crrev.com/cb4cf0bf96ffe158db13a5066cb2670560a86720/third_party/blink/renderer/modules/canvas/offscreencanvas2d/offscreen_canvas_rendering_context_2d.cc [modify] https://crrev.com/cb4cf0bf96ffe158db13a5066cb2670560a86720/third_party/blink/renderer/modules/canvas/offscreencanvas2d/offscreen_canvas_rendering_context_2d.h [modify] https://crrev.com/cb4cf0bf96ffe158db13a5066cb2670560a86720/third_party/blink/renderer/modules/csspaint/paint_rendering_context_2d.h [modify] https://crrev.com/cb4cf0bf96ffe158db13a5066cb2670560a86720/third_party/blink/renderer/modules/shapedetection/shape_detector.cc [modify] https://crrev.com/cb4cf0bf96ffe158db13a5066cb2670560a86720/third_party/blink/renderer/modules/webgl/webgl_rendering_context_base.cc
,
Dec 19
The beta bot result looks good. FirstRunMasterPrefsVariationsSeedTest.Test is failing but it was already failing before the merge. Unfortunately the bot doesn't run webkit layout tests though. Marking fixed. The fix should roll out to the next 72 Beta and the 73 Dev/Canary builds since 73.0.3640.0.
,
Dec 19
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/cb4cf0bf96ffe158db13a5066cb2670560a86720 Commit: cb4cf0bf96ffe158db13a5066cb2670560a86720 Author: falken@chromium.org Commiter: falken@chromium.org Date: 2018-12-18 23:23:57 +0000 UTC M72: Revert "Remove the "origin" parameter from ImageResource::IsAccessAllowed" This reverts commit 04dc53eaaac123d2dff125aba572d4195d095725. Reason: The change broke some extensions. Bug: 915384 Change-Id: Idae9fba283cfb5a904a6bc96c73cfb94cd1d64b2 Reviewed-on: https://chromium-review.googlesource.com/c/1381398 Reviewed-by: Matt Falkenhagen <falken@chromium.org> Reviewed-by: Yutaka Hirano <yhirano@chromium.org> Cr-Commit-Position: refs/branch-heads/3626@{#457} Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437} |
|||||||||||
►
Sign in to add a comment |
|||||||||||
Comment 1 by carlosil@chromium.org
, Dec 14Labels: -Type-Bug-Security -Restrict-View-SecurityTeam Type-Bug