Issue 294129: Canvas is tainted after drawing SVG including <foreignObject>

Hey Philip et al., what are the odds that we could adjust SVGImage::currentFrameHasSingleSecurityOrigin[1] to do something clever processing <foreignObject> rather than always tainting the canvas? :)

[1]: https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit/Source/core/svg/graphics/SVGImage.cpp&sq=package:chromium&q=currentFrameHasSingleSecurityOrigin&type=cs&l=99

I would love to land this change but we are a long way off from being able to not taint the canvas with foreignObject. The basic issue is that regular html leaks user data all over the place and our system is not designed to sandbox these cases.

Can we talk about what clever processing you might be looking for, or what usecase you need foreignObject for?

I am authoring a polyfill for a canvas API the browser lacks (aka "prollyfill") that will allow users to draw HTML to the canvas (read "ctx.drawHTML()").

My own use case is in-browser "screenshots" of web pages to facilitate UI testing via image diffs.

The effort is hosted under https://github.com/cburgmer/rasterizeHTML.js and is slowly finding an audience of web developers that want to do more with the canvas.

The basic technic is described in http://robert.ocallahan.org/2011/11/drawing-dom-content-to-canvas.html and is supported by Chrome, too, but Chrome falls short once trying to access the rendered content.

Foreign Object does not taint the canvas in firefox http://people.mozilla.org/~roc/rendering-HTML-elements-to-canvas.html

I'd like to add my reasons for an up-vote on this issue. As web apps become more prominent, there begins to be a need for interfacing with hardware. As in my case, we are building an app that allows users to print to a thermal printer (via java applet). While the printers cannot render an SVG image url, it can interpret a png image url.  My reasons for an upvote on this issue would suffice my needs for printing rendered html to a thermal printer using SVG. I understand this one reason does not justify pursuing this fix, but I wanted to answer any lingering, "why would anyone need this?"

I'd like to add my input here. I want to render html to a canvas, then use the canvas as a texture in a three.js scene (using css3d to render the pages has a big performance hit), so I'd really like this. I'm going to use html2canvas in the meantime.

It seems that in the meantime the canvas is not tainted if the same picture has been loaded from a data URI.

This provides for a workaround.

To further elaborate on the #7 comment. You can just use a data uri instead of loading the svg from a blob.  See the edited jsfiddle http://jsfiddle.net/KUH89/2/ also see this example http://people.mozilla.org/~roc/rendering-HTML-elements-to-canvas.html

Performance for using Blob URLs is scales of magnitude more performant than data URIs, per this jsperf: http://jsperf.com/loading-images-blobs-vs-dataurls/6

This kind of performance is essential for perf-sensitive cases like using canvas images as maps in WebGL. I really hope this gets resolved soon.

This is spec'ed to work so I think it's more of a bug. There are so many foreignobject issues that it's almost unusable, but I hope we can actually look into this area sometime in the future.

This appear to be working now.

@pdr: Is this a duplicate of some other issue that got fixed?

#17, we still don't support drawing foreginObject into a canvas without tainting it. I hope we can one day.

This bug was recently tweeted by SVG expert Sara Soueidan so I wanted to share my thoughts on it. Overall, we're making progress on fixing foreignObject but it'll take quite a bit of time.

Not tainting the canvas introduces a new privacy attack vector since it'll be the first time you can get form control / OS info / etc in javascript by reading the pixels from a canvas. I think this problem is tractable, but it's not easy.

Secondly, foreignObject is incredibly broken today in all browsers. Blink/Chromium have some fundamental flaws that will be addressed by the slimming paint project (https://www.chromium.org/blink/slimming-paint) but I think it'll be at least 6 months until that's completed. Given limited resources, I think it makes sense to fix the foreignObject issues before this bug.

So... progress, slowly but surely.

> foreignObject is incredibly broken today in all browsers

sorry, but not true. maybe my usage happened to miss all its bugs by chance, but if during development of one application i hit 4 blink/webkits bugs and only 1 gecko bug*, that hints at some browsers being more broken than others here.

* which was identical to one of the blink/webkit bugs, and had less impact than any of the 3 other blink/webkit bugs, *and* is easy to work around by adding transform="translate(0 0)" to the foreignObject.

fs and junov, I've been working on a related bug and am curious about your thoughts here. A recent article (https://goo.gl/78PwDy) reiterated how dangerous it is to get this wrong.

There are three attack vectors I'm aware of:
1) visited link data (aka the user's history)
I made some simple tests and we seem to be pretty good about not leaking visited link info these days. We would need to do a more thorough analysis here.

2) spellcheck dictionary (No red squigglies under guangdong? You might live in china)
It looks like we don't underline these until after the user actually types?

3) os theme
I'm not sure how much information would really be leaked here. On most platforms we draw non-native ui widgets. On OSX we do draw themed OS widgets which have some customization options that would be leaked.

Are there other attack vectors from allowing the reading of pixel data from a page? Within an svg image, the page in foreignObject is not able to make network requests or call javascript.

About your concern over OS theme (or browser theme and settings?): could be used to detect users with visual impairments (high contrast settings, color blindness settings, content magnification, etc.)

In #21 you did not mention cross-origin content (which is obvious), but let's not forget that this is a privacy attack vector that reaches way beyond fingerprinting. The presence of any cross-origin content in the foreignObject can be problematic. For general HTML rendering, the notion of cross-origin would go way beyond the case documented in the CRC2D.drawImage spec. Examples of cross-origin graphical content that may be present in an HTML document: tainted canvases, images, videos, iframes, fonts, stylesheets.
Leaking this sort of data could potentially allow a websites to spy on the ads that are served to it's users by third party ad networks (to infer all sorts of things). They could also spy on the state of like/+1 buttons, etc.

There are a number of embed-able HTML renderers out there (html2canvas, PhantomJS, etc.) Currently they are limited to whatever is observable from script, which makes them safe with respect to content security. Reading about the limitations of these libraries gives good ideas about what needs to be disallowed.

SVG in <img> is very strict about what resources it allows (more than SOP - https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit/Source/core/loader/FrameFetchContext.cpp&sq=package:chromium&l=526), so cross-origin loads should be covered. Similarly scripting is not enabled. The former might be subject to relaxation, but I can't imagine that will be less strict than SOP. So, I think pdr was interested in things beyond what those enable.

For the os theme, the worry is primarily fingerprinting. As a small example, I've set my OSX theme colors to be light blue on my work machine, and grey on my personal machine; with this change we would start leaking that. I am less worried about leaking accessibility information because we have to leak things like accessibility text size adjustments to let sites build great experiences for those users.

I don't like adding more fingerprinting bits but I think the usecases enabled by this change would be worth it.


The security side is less clear. fs is correct in #23, svg images are already locked down and have no access to the network. Having done some backflips to try to get data into SVGImage.cpp, I'm confident the network sandbox protections of svg image are strong. The real danger is leaking visited link information which is managed in the browser process. In my tests, this doesn't leak, but I don't think we can say that confidently yet.

I heard a rumor that Alex Russel (slightlyoff) is working on spec language to lock down visited links to only report themselves as "visited" when the links have been visited on the current origin.

No promises or timeline, but there's a chance that his work will unblock this highly-starred bug :)

You seem to only taint the canvas when the svg comes from a BlobURI, when it comes from a dataURI it is not tainted : https://jsfiddle.net/2Lh24rg9/

IIRC, canvas has a special case for data URLs (but not for blob URLs.)

@f...@opera.com so this means that you are able to protect user privacy with dataURIs but not with blobs ? o_O
Or should I open an other bug report about it ? I mean it's a security issue right ?

I don't think too much should be read into that... That discrepancy likely stem from how data URLs have been (and are) unique origins in WebKit/Blink, while other UAs supported the use of data URLs in these contexts. Consider me more an archaeologist here though, I don't know to what degree one or the other has been considered... Please do file a bug about it, and then maybe we'll know =).

The blob URI vs data URI discrepancy is a coding error IMHO. The origin check exception we are making for data URIs assumes a single security origin so it goes around the mechanism we use to check for foreignObject tags.

Shall we lift the restriction on foreignObject nodes? As pointed out in #24, the main concern at this point is fingerprinting. Users who require extra protection against fingerprinting should start Chrome with "--disable-reading-from-canvas" which will block this as well as any other canvas-based fingerprinting vector.

Lifting the foreignObject restriction will enable some use cases and improve browser interoperability.

Another argument for lifting the foreignObject restriction is that it won't actually make the browser more vulnerable than it is today since the restriction was already circumventable by going through a Data URI, as pointed out in #26.

TODO:

* Add a test that verifies that external resources (i.e. that are not inlined in the SVG) are not loaded. That test will be a trip wire in case we enable the loading of those resources in the future, to make sure we are careful about not leaking cross-origin content via untainted canvases.

* Add comments to svg/as-image/svg-canvas-link-not-colored.html to document that this test is a safeguard against a data security issue.

* Add a web-platform-test to track this currently non-interoperable behavior.

Issue 742548 has been merged into this issue.

Hi,

Did this end up shipping?

chrishtr: Nope. I just realized I was mistaken in a tweet I made yesterday that referenced this issue being fixed. Sorry about that if that's why you're here (@sephr)

It looks like WebKit is about to land this change:
https://bugs.webkit.org/show_bug.cgi?id=180301

Would be great if we could as well.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/91ae23a52889d7d2e09b328de03918168a01f798

commit 91ae23a52889d7d2e09b328de03918168a01f798
Author: Matt Falkenhagen <falken@chromium.org>
Date: Wed Jan 09 01:33:06 2019

canvas: Restore the data: URL special case for tainting.

CanvasRenderingContext::WouldTaintOrigin() had a special case for data
URLs that was removed in r610498.[1] The assumption was that just calling
CanvasImageSource::WouldTaintOrigin() would return false on data URLs.
It turns out that function can return true due to a historical
restriction on SVG foreign object nodes, as discussed in  bug 294129 .

This CL reverses that behavior change, so data URLs again don't taint
the canvas. It partially reverts r610498 and dependent change r613433.

A WPT test is added. Chrome now passes the test despite  bug 294129  being
open because it has this special case for data URLs on canvas.

[1] https://chromium-review.googlesource.com/c/chromium/src/+/1347953

Bug:  294129 ,  918460 
Change-Id: I7c8cb4d37d950693956785c291dfd7660c42e662
Reviewed-on: https://chromium-review.googlesource.com/c/1400433
Reviewed-by: Kenneth Russell <kbr@chromium.org>
Commit-Queue: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#620985}
[modify] https://crrev.com/91ae23a52889d7d2e09b328de03918168a01f798/third_party/blink/renderer/core/html/canvas/canvas_image_source.h
[modify] https://crrev.com/91ae23a52889d7d2e09b328de03918168a01f798/third_party/blink/renderer/core/html/canvas/canvas_rendering_context.cc
[modify] https://crrev.com/91ae23a52889d7d2e09b328de03918168a01f798/third_party/blink/renderer/core/html/canvas/image_element_base.cc
[modify] https://crrev.com/91ae23a52889d7d2e09b328de03918168a01f798/third_party/blink/renderer/core/html/canvas/image_element_base.h
[modify] https://crrev.com/91ae23a52889d7d2e09b328de03918168a01f798/third_party/blink/renderer/core/html/media/html_video_element.h
[add] https://crrev.com/91ae23a52889d7d2e09b328de03918168a01f798/third_party/blink/web_tests/external/wpt/2dcontext/drawing-images-to-the-canvas/drawimage_svg_image_with_foreign_object_does_not_taint.html

The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/22de51b5fecebdc776936494ef4b6c8edd94acd0

Commit: 22de51b5fecebdc776936494ef4b6c8edd94acd0
Author: falken@chromium.org
Commiter: falken@chromium.org
Date: 2019-01-10 02:05:16 +0000 UTC

M72: canvas: Restore the data: URL special case for tainting.

CanvasRenderingContext::WouldTaintOrigin() had a special case for data
URLs that was removed in r610498.[1] The assumption was that just calling
CanvasImageSource::WouldTaintOrigin() would return false on data URLs.
It turns out that function can return true due to a historical
restriction on SVG foreign object nodes, as discussed in  bug 294129 .

This CL reverses that behavior change, so data URLs again don't taint
the canvas. It partially reverts r610498 and dependent change r613433.

A WPT test is added. Chrome now passes the test despite  bug 294129  being
open because it has this special case for data URLs on canvas.

[1] https://chromium-review.googlesource.com/c/chromium/src/+/1347953

(cherry picked from commit 91ae23a52889d7d2e09b328de03918168a01f798)

Bug:  294129 ,  918460 
Change-Id: I7c8cb4d37d950693956785c291dfd7660c42e662
Reviewed-on: https://chromium-review.googlesource.com/c/1400433
Reviewed-by: Kenneth Russell <kbr@chromium.org>
Commit-Queue: Matt Falkenhagen <falken@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#620985}
Reviewed-on: https://chromium-review.googlesource.com/c/1404537
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/branch-heads/3626@{#630}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/22de51b5fecebdc776936494ef4b6c8edd94acd0

commit 22de51b5fecebdc776936494ef4b6c8edd94acd0
Author: Matt Falkenhagen <falken@chromium.org>
Date: Thu Jan 10 02:05:16 2019

M72: canvas: Restore the data: URL special case for tainting.

CanvasRenderingContext::WouldTaintOrigin() had a special case for data
URLs that was removed in r610498.[1] The assumption was that just calling
CanvasImageSource::WouldTaintOrigin() would return false on data URLs.
It turns out that function can return true due to a historical
restriction on SVG foreign object nodes, as discussed in  bug 294129 .

This CL reverses that behavior change, so data URLs again don't taint
the canvas. It partially reverts r610498 and dependent change r613433.

A WPT test is added. Chrome now passes the test despite  bug 294129  being
open because it has this special case for data URLs on canvas.

[1] https://chromium-review.googlesource.com/c/chromium/src/+/1347953

(cherry picked from commit 91ae23a52889d7d2e09b328de03918168a01f798)

Bug:  294129 ,  918460 
Change-Id: I7c8cb4d37d950693956785c291dfd7660c42e662
Reviewed-on: https://chromium-review.googlesource.com/c/1400433
Reviewed-by: Kenneth Russell <kbr@chromium.org>
Commit-Queue: Matt Falkenhagen <falken@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#620985}
Reviewed-on: https://chromium-review.googlesource.com/c/1404537
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/branch-heads/3626@{#630}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}
[modify] https://crrev.com/22de51b5fecebdc776936494ef4b6c8edd94acd0/third_party/blink/renderer/core/html/canvas/canvas_rendering_context.cc
[add] https://crrev.com/22de51b5fecebdc776936494ef4b6c8edd94acd0/third_party/blink/web_tests/external/wpt/2dcontext/drawing-images-to-the-canvas/drawimage_svg_image_with_foreign_object_does_not_taint.html