Bug 119492 – Implement SVGImage::hasSingleSecurityOrigin()

• Status: RESOLVED
• Resolution: FIXED
• Component: SVG

[Timothy Hatcher]

By not implementing hasSingleSecurityOrigin(), any SVG rendered into a canvas will taint the canvas and prevent getting the pixels or image data. We should implement a basic version of hasSingleSecurityOrigin() that allows simple SVGs to not taint a canvas.

[Philip Rogers]

Curious: do you have a plan for handling cross-origin visited link colors (<a>) and foreignObject?

[Timothy Hatcher]

My interest (Web Inspector) only cares about simple SVGs, nothing interactive. No linked content, just paths really.

[Timothy Hatcher]

Created attachment 208235 [details]
Proposed Change (WIP)

Here is an attempt at getting this working for simple SVGs. This works for the images the Inspector uses.

Any thoughts? Am I completely off base here?

[Philip Rogers]

Comment on attachment 208235 [details]
Proposed Change (WIP)

SVG images (<img src="svg.svg">) shouldn't be able to load external resources to begin with (even same origin!), so these checks do not seem appropriate. Furthermore, a one-time static check can be circumvented using SMIL; the following svg image will pass the selector check but will still contain an xlink:href:
<?xml version="1.0" encoding="UTF-8" standalone="no"?>
<!DOCTYPE svg  PUBLIC '-//W3C//DTD SVG 1.1//EN'  'http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd'>
<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" width="200" height="200">
  <rect id="rect" x="100" y="100" width="100" height="100" fill="green"/>
  <use x="-100" y="-100" width="50" height="50" transform="rotate(10)">
    <set attributeName="xlink:href" to="#rect" begin="0s"/>
  </use>
</svg>


My understanding of the security threat here is that svg images can leak cross origin data through <a> (due to the link highlight color), foreignObject, and possibly tref. Disabling these explicitly may be a better approach.

[Timothy Hatcher]

(In reply to comment #4)
> (From update of attachment 208235 [details])
> SVG images (<img src="svg.svg">) shouldn't be able to load external resources to begin with (even same origin!), so these checks do not seem appropriate.

Okay, that simplifies things. (FWIW, external resources use to load in SVG images in Safari 6, but no longer do in TOT.)

> Furthermore, a one-time static check can be circumvented using SMIL; the following svg image will pass the selector check but will still contain an xlink:href.

This is not a static check. It happens each time the SVG is drawn into the canvas, so the SMIL approach changes nothing. It still gets caught.

> My understanding of the security threat here is that svg images can leak cross origin data through <a> (due to the link highlight color), foreignObject, and possibly tref. Disabling these explicitly may be a better approach.

Link visited color I get. <foreignObject> I get if it contains XHTML which can contain links too. I don't see how <tref> can be a problem.

My gut now says checking for "*|a[*|href]" elements is enough, even for XHTML inside <foreignObject>. But I am an SVG noob, please, correct me if I am wrong.

[Timothy Hatcher]

Created attachment 208263 [details]
Proposed Change (WIP 2)

[Timothy Hatcher]

The other option would be to have a setting to force links to be unvisited and use that for SVGImage's Page.

[Timothy Hatcher]

Created attachment 208292 [details]
Patch

[Darin Adler]

Comment on attachment 208292 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=208292&action=review

> Source/WebCore/svg/graphics/SVGImage.cpp:286
> +    DEFINE_STATIC_LOCAL(AtomicString, questionableElementsSelector, ("*|a[*|href]", AtomicString::ConstructFromLiteral));
> +    RefPtr<Element> questionableElement = rootElement->querySelector(questionableElementsSelector, ASSERT_NO_EXCEPTION);

I’d rather see a hand-written loop than use of querySelector here. The strange need for an atomic string and such. Any good reason not to write the loop? It's a simple traversal with a hasTagName and hasAttribute check on each element.

[Timothy Hatcher]

(In reply to comment #9)
> (From update of attachment 208292 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=208292&action=review
> 
> > Source/WebCore/svg/graphics/SVGImage.cpp:286
> > +    DEFINE_STATIC_LOCAL(AtomicString, questionableElementsSelector, ("*|a[*|href]", AtomicString::ConstructFromLiteral));
> > +    RefPtr<Element> questionableElement = rootElement->querySelector(questionableElementsSelector, ASSERT_NO_EXCEPTION);
> 
> I’d rather see a hand-written loop than use of querySelector here. The strange need for an atomic string and such. Any good reason not to write the loop? It's a simple traversal with a hasTagName and hasAttribute check on each element.

querySelector is smarter and cached, so repeated calls will have better performance than a loop of all the elements.

[Build Bot]

Comment on attachment 208292 [details]
Patch

Attachment 208292 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/1370885

New failing tests:
http/tests/security/canvas-remote-read-svg-image.html
fast/canvas/svg-taint.html
http/tests/security/canvas-remote-read-data-url-svg-image.html

[Build Bot]

Created attachment 208294 [details]
Archive of layout-test-results from webkit-ews-01 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-01  Port: mac-mountainlion  Platform: Mac OS X 10.8.4

[Joseph Pecoraro]

Comment on attachment 208292 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=208292&action=review

Looks good to me. Tim also answered Darin's question.

>>> Source/WebCore/svg/graphics/SVGImage.cpp:286
>>> +    RefPtr<Element> questionableElement = rootElement->querySelector(questionableElementsSelector, ASSERT_NO_EXCEPTION);
>> 
>> I’d rather see a hand-written loop than use of querySelector here. The strange need for an atomic string and such. Any good reason not to write the loop? It's a simple traversal with a hasTagName and hasAttribute check on each element.
> 
> querySelector is smarter and cached, so repeated calls will have better performance than a loop of all the elements.

It looks like querySelector only caches the "SelectorQuery" object, but doesn't cache seem to cache anything else. selectorQuery->queryFirst(…) looks like it would still always happen and walk the tree. So I'm not sure repeated queries would actually perform better. So Darin's comment still seems reasonable.

[Timothy Hatcher]

Created attachment 208301 [details]
Patch

[Philip Rogers]

I think this is heading in the right direction, but a few issues remain.


> This is not a static check. It happens each time the SVG is drawn into the canvas, so the SMIL approach changes nothing. It still gets caught.
Did you try the example I provided?


> My gut now says checking for "*|a[*|href]" elements is enough, even for XHTML inside <foreignObject>. But I am an SVG noob, please, correct me if I am wrong.

Because href is useful in even simple svg examples, I would recommend blacklisting <foreignObject> instead of the href attribute.

Firefox may have something to teach us here about <a>. Firefox does not highlight link colors in svg images at all; this is doable by checking if we're an image in SVGAElement.cpp before setting setIsLink. I think this may be a better approach than walking the dom looking for links.

Lastly, subimages are another issue. You'll want to recursively walk all <image>s to ensure they all have a single origin as well. Otherwise, someone could embed a data uri svg image that itself contains <a> elements.

[Timothy Hatcher]

(In reply to comment #15)
> I think this is heading in the right direction, but a few issues remain.
> 
> 
> > This is not a static check. It happens each time the SVG is drawn into the canvas, so the SMIL approach changes nothing. It still gets caught.
> Did you try the example I provided?
> 
> 
> > My gut now says checking for "*|a[*|href]" elements is enough, even for XHTML inside <foreignObject>. But I am an SVG noob, please, correct me if I am wrong.
> 
> Because href is useful in even simple svg examples, I would recommend blacklisting <foreignObject> instead of the href attribute.

You can have links in SVG without <foreignObject>. I was blocking <a href> not other useful uses of href. My first patch did block all href uses, and that was wrong. My latest patch blocks any link element.

> Firefox may have something to teach us here about <a>. Firefox does not highlight link colors in svg images at all; this is doable by checking if we're an image in SVGAElement.cpp before setting setIsLink. I think this may be a better approach than walking the dom looking for links.

That could work. But we would also need to block <foreignObject> or do similar in HTMLAnchorElement.

> Lastly, subimages are another issue. You'll want to recursively walk all <image>s to ensure they all have a single origin as well. Otherwise, someone could embed a data uri svg image that itself contains <a> elements.

Good point.

[Philip Rogers]

(In reply to comment #16)
> (In reply to comment #15)
> > I think this is heading in the right direction, but a few issues remain.
> > 
> > 
> > > This is not a static check. It happens each time the SVG is drawn into the canvas, so the SMIL approach changes nothing. It still gets caught.
> > Did you try the example I provided?
> > 
> > 
> > > My gut now says checking for "*|a[*|href]" elements is enough, even for XHTML inside <foreignObject>. But I am an SVG noob, please, correct me if I am wrong.
> > 
> > Because href is useful in even simple svg examples, I would recommend blacklisting <foreignObject> instead of the href attribute.
> 
> You can have links in SVG without <foreignObject>. I was blocking <a href> not other useful uses of href. My first patch did block all href uses, and that was wrong. My latest patch blocks any link element.
> 
> > Firefox may have something to teach us here about <a>. Firefox does not highlight link colors in svg images at all; this is doable by checking if we're an image in SVGAElement.cpp before setting setIsLink. I think this may be a better approach than walking the dom looking for links.
> 
> That could work. But we would also need to block <foreignObject> or do similar in HTMLAnchorElement.
> 
> > Lastly, subimages are another issue. You'll want to recursively walk all <image>s to ensure they all have a single origin as well. Otherwise, someone could embed a data uri svg image that itself contains <a> elements.
> 
> Good point.

Awesome, I think we're on the same page. Just to make sure, is this the current plan?
1) Block <foreignObject>.
2) Recursively check that subimages are from the same origin.
3) Block <a> either by not coloring it like a link, or by walking the tree.

[Timothy Hatcher]

(In reply to comment #17)
> (In reply to comment #16)
> > (In reply to comment #15)
> > > I think this is heading in the right direction, but a few issues remain.
> > > 
> > > 
> > > > This is not a static check. It happens each time the SVG is drawn into the canvas, so the SMIL approach changes nothing. It still gets caught.
> > > Did you try the example I provided?
> > > 
> > > 
> > > > My gut now says checking for "*|a[*|href]" elements is enough, even for XHTML inside <foreignObject>. But I am an SVG noob, please, correct me if I am wrong.
> > > 
> > > Because href is useful in even simple svg examples, I would recommend blacklisting <foreignObject> instead of the href attribute.
> > 
> > You can have links in SVG without <foreignObject>. I was blocking <a href> not other useful uses of href. My first patch did block all href uses, and that was wrong. My latest patch blocks any link element.
> > 
> > > Firefox may have something to teach us here about <a>. Firefox does not highlight link colors in svg images at all; this is doable by checking if we're an image in SVGAElement.cpp before setting setIsLink. I think this may be a better approach than walking the dom looking for links.
> > 
> > That could work. But we would also need to block <foreignObject> or do similar in HTMLAnchorElement.
> > 
> > > Lastly, subimages are another issue. You'll want to recursively walk all <image>s to ensure they all have a single origin as well. Otherwise, someone could embed a data uri svg image that itself contains <a> elements.
> > 
> > Good point.
> 
> Awesome, I think we're on the same page. Just to make sure, is this the current plan?
> 1) Block <foreignObject>.
> 2) Recursively check that subimages are from the same origin.
> 3) Block <a> either by not coloring it like a link, or by walking the tree.

If you do 3, you shouldn't need to do 2, right? You also don't need to do 1 if HTMLAnchorElement in SVGImage does not color.

[Timothy Hatcher]

Comment on attachment 208301 [details]
Patch

I have this working differently now. It is very simple to prevent SVG and HTML links from rendering as links when in an SVGImage. I just need to redo the tests now that even links are allowed and there is no tainting at all for SVG.

[Timothy Hatcher]

Created attachment 208340 [details]
Patch

[Timothy Hatcher]

Data URL SVG images are not allowed. If they were, this latest patch would still work.

[Philip Rogers]

I like where this patch is going! One issue that remains is <foreignObject>: I think we should still disable this completely because it opens a whole can of security worms since fO can contain almost arbitrary html.

(In reply to comment #21)
> Data URL SVG images are not allowed. If they were, this latest patch would still work.

Unfortunately this is a bit of a security red herring. There's a bug where you can pre-seed the memorycache and get data uri images to work in SVG by loading the image twice. The Swiffy Flash->SVG converter uses the preseed technique to workaround wkbug.com/118068.

[Darin Adler]

Comment on attachment 208340 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=208340&action=review

Looks good. review- because of change log, but I also suggest doing this without touching Element.h/cpp

> Source/WebCore/ChangeLog:15
> +        (WebCore::SVGImage::hasSingleSecurityOrigin): Added. Look for links and return
> +        false if any link is found.

This no longer matches the patch. Need a revised change log.

> Source/WebCore/dom/Element.h:592
> +    bool isEmbeddedThroughSVGImage() const;

It’s great to have this be a function, but I don’t think it’s good as a member function. Can we make it a free function in HTMLAnchorElement.h/cpp and have the other files include HTMLAnchorElement.h? Or something similar in some SVG source file? Also, maybe we could name it for its effect rather than for what it checks, shouldDisableLinks or shouldProhibitLinks?

> Source/WebCore/svg/graphics/SVGImage.h:57
> +    // SVG images disallow external resources (including same origin).
> +    virtual bool hasSingleSecurityOrigin() const OVERRIDE { return true; }

The comment here is too oblique, I think. Maybe something more like this?

    // Because SVG image rendering disallows external resources and links, these images effectively are restricted to a single security origin.

[Timothy Hatcher]

(In reply to comment #22)
> I like where this patch is going! One issue that remains is <foreignObject>: I think we should still disable this completely because it opens a whole can of security worms since fO can contain almost arbitrary html.

I disagree that foreignObject needs blocked, unless you have a concrete example of an issue.

[Timothy Hatcher]

Comment on attachment 208340 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=208340&action=review

>> Source/WebCore/ChangeLog:15
>> +        false if any link is found.
> 
> This no longer matches the patch. Need a revised change log.

Opps.

>> Source/WebCore/dom/Element.h:592
>> +    bool isEmbeddedThroughSVGImage() const;
> 
> It’s great to have this be a function, but I don’t think it’s good as a member function. Can we make it a free function in HTMLAnchorElement.h/cpp and have the other files include HTMLAnchorElement.h? Or something similar in some SVG source file? Also, maybe we could name it for its effect rather than for what it checks, shouldDisableLinks or shouldProhibitLinks?

OK, I started down that road but ended up with this. I'm fine making it standalone. I kept the isEmbeddedThroughSVGImage name so it could be shared by RenderSVGRoot.cpp, which is unrelated to links.

>> Source/WebCore/svg/graphics/SVGImage.h:57
>> +    virtual bool hasSingleSecurityOrigin() const OVERRIDE { return true; }
> 
> The comment here is too oblique, I think. Maybe something more like this?
> 
>     // Because SVG image rendering disallows external resources and links, these images effectively are restricted to a single security origin.

OK.

[Timothy Hatcher]

(In reply to comment #22)
> I like where this patch is going! One issue that remains is <foreignObject>: I think we should still disable this completely because it opens a whole can of security worms since fO can contain almost arbitrary html.

Scripts and sub-resource loading are all disabled in SVGImage. That applies to <foreignObject> too.

[Timothy Hatcher]

Created attachment 208358 [details]
Patch

[Darin Adler]

Comment on attachment 208358 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=208358&action=review

review- because I still would prefer we not touch Element.h

> Source/WebCore/dom/Element.h:802
> +bool isElementContainedInSVGImage(const Element*);

This is the wrong header file. I suggest putting it into SVGImage.h or HTMLAnchorElement.h since it’s about SVG images and links.

One of the points of having something be a free function instead of a member function is that we can encapsulate ideas better and not pollute the core classes with all the more complex concepts.

It would be better to have this function name talk about link policy, rather than the SVG Image check. That would make the reason for the check at all the call sites clearer. This function would then be the one place to add comments explaining the policy. Otherwise, at each call site we’d raise the question, “Why is an SVG image special here?” And then have to add comments everywhere.

If you don’t agree, and want to keep something more like the current name, I would use isInSVGImage; I don’t think the words “element contained” are needed.

> Source/WebCore/svg/graphics/SVGImage.cpp:38
> +#include "NodeTraversal.h"

Why?

[Darin Adler]

Comment on attachment 208358 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=208358&action=review

>> Source/WebCore/dom/Element.h:802
>> +bool isElementContainedInSVGImage(const Element*);
> 
> This is the wrong header file. I suggest putting it into SVGImage.h or HTMLAnchorElement.h since it’s about SVG images and links.
> 
> One of the points of having something be a free function instead of a member function is that we can encapsulate ideas better and not pollute the core classes with all the more complex concepts.
> 
> It would be better to have this function name talk about link policy, rather than the SVG Image check. That would make the reason for the check at all the call sites clearer. This function would then be the one place to add comments explaining the policy. Otherwise, at each call site we’d raise the question, “Why is an SVG image special here?” And then have to add comments everywhere.
> 
> If you don’t agree, and want to keep something more like the current name, I would use isInSVGImage; I don’t think the words “element contained” are needed.

Oops, I missed your comment about RenderSVGRoot. I think that SVGImage.h should have the isInSVGImage, and HTMLAnchorElement.h should have shouldProhibitLinks or shouldAllowLinks, and the second function should call the first.

[Darin Adler]

Comment on attachment 208358 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=208358&action=review

>>> Source/WebCore/dom/Element.h:802
>>> +bool isElementContainedInSVGImage(const Element*);
>> 
>> This is the wrong header file. I suggest putting it into SVGImage.h or HTMLAnchorElement.h since it’s about SVG images and links.
>> 
>> One of the points of having something be a free function instead of a member function is that we can encapsulate ideas better and not pollute the core classes with all the more complex concepts.
>> 
>> It would be better to have this function name talk about link policy, rather than the SVG Image check. That would make the reason for the check at all the call sites clearer. This function would then be the one place to add comments explaining the policy. Otherwise, at each call site we’d raise the question, “Why is an SVG image special here?” And then have to add comments everywhere.
>> 
>> If you don’t agree, and want to keep something more like the current name, I would use isInSVGImage; I don’t think the words “element contained” are needed.
> 
> Oops, I missed your comment about RenderSVGRoot. I think that SVGImage.h should have the isInSVGImage, and HTMLAnchorElement.h should have shouldProhibitLinks or shouldAllowLinks, and the second function should call the first.

Maybe not literally "SVGImage.h", but whatever file we have that is closest to that.

[Timothy Hatcher]

Created attachment 208363 [details]
Patch

[Timothy Hatcher]

Comment on attachment 208358 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=208358&action=review

>>>> Source/WebCore/dom/Element.h:802
>>>> +bool isElementContainedInSVGImage(const Element*);
>>> 
>>> This is the wrong header file. I suggest putting it into SVGImage.h or HTMLAnchorElement.h since it’s about SVG images and links.
>>> 
>>> One of the points of having something be a free function instead of a member function is that we can encapsulate ideas better and not pollute the core classes with all the more complex concepts.
>>> 
>>> It would be better to have this function name talk about link policy, rather than the SVG Image check. That would make the reason for the check at all the call sites clearer. This function would then be the one place to add comments explaining the policy. Otherwise, at each call site we’d raise the question, “Why is an SVG image special here?” And then have to add comments everywhere.
>>> 
>>> If you don’t agree, and want to keep something more like the current name, I would use isInSVGImage; I don’t think the words “element contained” are needed.
>> 
>> Oops, I missed your comment about RenderSVGRoot. I think that SVGImage.h should have the isInSVGImage, and HTMLAnchorElement.h should have shouldProhibitLinks or shouldAllowLinks, and the second function should call the first.
> 
> Maybe not literally "SVGImage.h", but whatever file we have that is closest to that.

I put shouldProhibitLinks on Element so the three subclasses can all use it, not just HTMLAnchorElement.

>> Source/WebCore/svg/graphics/SVGImage.cpp:38
>> +#include "NodeTraversal.h"
> 
> Why?

Forgot to remove this.

[Philip Rogers]

(In reply to comment #24)
> (In reply to comment #22)
> > I like where this patch is going! One issue that remains is <foreignObject>: I think we should still disable this completely because it opens a whole can of security worms since fO can contain almost arbitrary html.
> 
> I disagree that foreignObject needs blocked, unless you have a concrete example of an issue.

You can leak the user's theme by drawing a button in a foreign object and then reading it out of the canvas. If you care about the user's spellcheck dictionary, you may be able to coax an input box to underline a misspelled word. I'm not sure if these are serious issues, but these haven't been exposed before.

I'm cc'ing inferno and cevans in case they can think of other attack vectors.

[Chris Evans]

"I disagree that foreignObject needs blocked, unless you have a concrete example of an issue."

This seems like a grenade that isn't worth playing with in this initial patch. I recommend we land with <foreignObject> blocked and then file a follow-up bug to iterate and improve, with input from webkit-security and other security experts.

Certainly, the Blink variant of this patch will be blocking <foreignObject>, so it'd be nice to be compatible. By also blocking <foreignObject>, you'll mitigate the risk that Safari ends up with a security bug that Chrome and Opera do not have.

[Darin Adler]

Comment on attachment 208363 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=208363&action=review

I’m sorry, not trying to be difficult, but I still say review-

> Source/WebCore/dom/Element.h:679
> +    bool shouldProhibitLinks() const;

I’m sorry to be a broken record, but I still do not think this should be a member function. I think a free function in HTMLAnchorElement.h would be better. The other link-hosting classes can include HTMLAnchorElement.h to get this.

[Timothy Hatcher]

Comment on attachment 208363 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=208363&action=review

>> Source/WebCore/dom/Element.h:679
>> +    bool shouldProhibitLinks() const;
> 
> I’m sorry to be a broken record, but I still do not think this should be a member function. I think a free function in HTMLAnchorElement.h would be better. The other link-hosting classes can include HTMLAnchorElement.h to get this.

No problem. I'll make it standalone.

[Timothy Hatcher]

(In reply to comment #34)
> "I disagree that foreignObject needs blocked, unless you have a concrete example of an issue."
> 
> This seems like a grenade that isn't worth playing with in this initial patch. I recommend we land with <foreignObject> blocked and then file a follow-up bug to iterate and improve, with input from webkit-security and other security experts.
> 
> Certainly, the Blink variant of this patch will be blocking <foreignObject>, so it'd be nice to be compatible. By also blocking <foreignObject>, you'll mitigate the risk that Safari ends up with a security bug that Chrome and Opera do not have.

I'll block it.

[Timothy Hatcher]

Created attachment 208393 [details]
Patch

[Darin Adler]

Comment on attachment 208393 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=208393&action=review

> Source/WebCore/svg/graphics/SVGImage.cpp:72
> +    Frame* frame = m_page->mainFrame();
> +    SVGSVGElement* rootElement = toSVGDocument(frame->document())->rootElement();

For the record, there was no need to put frame into a local variable.

[WebKit Commit Bot]

Comment on attachment 208393 [details]
Patch

Clearing flags on attachment: 208393

Committed r153876: <http://trac.webkit.org/changeset/153876>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Philip Rogers]

*** Bug 108755 has been marked as a duplicate of this bug. ***

[Frédéric Wang (:fredw)]

(In reply to comment #34)
> This seems like a grenade that isn't worth playing with in this initial patch. I recommend we land with <foreignObject> blocked and then file a follow-up bug to iterate and improve, with input from webkit-security and other security experts.
> 
> Certainly, the Blink variant of this patch will be blocking <foreignObject>, so it'd be nice to be compatible. By also blocking <foreignObject>, you'll mitigate the risk that Safari ends up with a security bug that Chrome and Opera do not have.

FYI, I've opened bug 131033 for MathML in <foreignObject>. The use case that I have in mind is to integrate mathematical formulas in 3D schemas. At the moment, with WebKit, I can not integrate them directly in the WebGL canvas (and have to use https://github.com/unconed/MathBox.js to position formulas above the canvas).

MathML can have links but this is not implemented yet (bug 85733). Gecko seems to block the color for MathML links when embedded in a WebGL canvas. I don't know see other security issues off the top of my head (except of course that HTML can itself be embedded in MathML, and so this should be handled too).