Bug 156176 - Investigate letting foreignObject not taint the canvas when drawing svg into canvas.
Summary: Investigate letting foreignObject not taint the canvas when drawing svg into ...
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: SVG (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks: 91523 131033
  Show dependency treegraph
 
Reported: 2016-04-04 13:18 PDT by Philip Rogers
Modified: 2018-02-13 09:55 PST (History)
11 users (show)

See Also:


Attachments
SVG (725 bytes, image/svg+xml)
2017-01-05 14:27 PST, Said Abou-Hallawa
no flags Details
test case (1.32 KB, text/html)
2017-01-05 14:28 PST, Said Abou-Hallawa
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Philip Rogers 2016-04-04 13:18:26 PDT
Out of an abundance of caution [1] webkit currently taints the canvas when an svg image containing a foreign object is drawn into the canvas[2]. The core issue is described in https://bugs.webkit.org/show_bug.cgi?id=119492#c33. Blink also has this behavior and we recently reconsidered it in https://crbug.com/294129#c21, but no progress has been made (I will update this bug if there ever is any).

I'd think we should change this, but it's risky and I haven't seen a lot of user interest in it.

[1] Getting this wrong has serious issues, see: https://goo.gl/78PwDy
[2] http://trac.webkit.org/browser/trunk/Source/WebCore/svg/graphics/SVGImage.cpp?rev=198655#L85
Comment 1 Frédéric Wang (:fredw) 2016-04-05 02:22:48 PDT
FWIW, the MDN page: https://developer.mozilla.org/en-US/docs/Web/API/Canvas_API/Drawing_DOM_objects_into_a_canvas
Comment 2 Philip Rogers 2016-04-05 11:02:22 PDT
@Frederic, on the webkit-dev thread you asked "Maybe it would be worth checking with them what was their rationale to remove that restriction and if it's worth following the same approach for Blink/WebKit...". I think we could remove this restriction today, but I do not trust our implementation of foreignObject to not leak data. This problem is specific to our implementation.

I think a path forward will be to have someone look very closely at <foreignObject> and the data it can leak, and then just flip the switch if it is safe (remove SVGImage::hasSingleSecurityOrigin). For WebKit, I would recommend asking someone on Apple's security team to sign off on this too. I support doing this, but it's risky; I haven't done it myself because there hasn't been enough user interest to justify it.
Comment 3 Philip Rogers 2016-12-20 19:04:38 PST
Simon and Said,
I think we're going to go ahead with this change in Blink (https://groups.google.com/a/chromium.org/d/msg/blink-dev/yYVVl5ociqA/b5387_fKDwAJ). I follow SVG commits in both Blink and WebKit and I do not know of any security/privacy differences in this area. Would you support the same change in WebKit? I can post the patch but wanted to check with you first.
Comment 4 Said Abou-Hallawa 2017-01-05 14:26:53 PST
(In reply to comment #3)
> Simon and Said,
> I think we're going to go ahead with this change in Blink
> (https://groups.google.com/a/chromium.org/d/msg/blink-dev/yYVVl5ociqA/
> b5387_fKDwAJ). I follow SVG commits in both Blink and WebKit and I do not
> know of any security/privacy differences in this area. Would you support the
> same change in WebKit? I can post the patch but wanted to check with you
> first.

I agree with this change since this will make WebKit compliant with the specs and the other browsers. I did a basic testing and I found out WebKt does not apply any linking style when drawing an SVG to a canvas (see attached test case). But I think the WebKit security team needs to sign off on this as well. Brent, do you agree with this change?
Comment 5 Said Abou-Hallawa 2017-01-05 14:27:19 PST
Created attachment 298134 [details]
SVG
Comment 6 Said Abou-Hallawa 2017-01-05 14:28:25 PST
Created attachment 298135 [details]
test case
Comment 7 Philip Rogers 2017-01-05 14:35:30 PST
Thanks Said! Small update on the blink side: junov is currently writing a few more tests just to be sure®. I'll update this bug (along with a link to the patch with tests) once the full change lands in blink.
Comment 8 Aman Vishnani 2018-02-13 09:55:47 PST
Hi there,

Any progress on this bug? I came across this bug while using the dom-to-image library for converting an HTML element into a sharable png image. I was hoping that we could make this work without restrictions since Google Chrome and Firefox already allows it.

This issue is similar to https://bugs.webkit.org/show_bug.cgi?id=17352