1126352 - XSS when pasting from other app into contenteditable field, with SVG <animate>

Status: VERIFIED FIXED

(Core :: DOM: Editor, defect)

Description

[Mario Heiderich]

Attachment: gmail-copypaste.odt

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:35.0) Gecko/20100101 Firefox/35.0
Build ID: 20150112203201

Steps to reproduce:

I open the attached ODT, then I open Firefox - and navigate to a page with editable content. This could be "html5sec.org/copypaste" ... or Gmail.

I copy the string "Hello" from the attached document. I paste it into an editable area. I do this very elegantly using Ctrl-A, Ctrl-C and then Ctrl-V. 

Then I click the resulting black SVG rectangle and an alert pops up.


Actual results:

Injected HTML from within the font-names of the fonts used in the ODT make it into the editable area, bypassing FF's sanitizer from cross-origin / cross-application rich-text transfer.

The actual payload looks like this, you will find the source of it inside the ODT in a file called styles.xml.

<div contenteditable="false"><svg><a xmlns:xlink="http://www.w3.org/1999/xlink" xlink:href="?"><circle r="4000"></circle><animate attributeName="xlink:href" begin="0" from="javascript:alert(document.domain)" to="&amp;"></animate></a></svg></div>

Note: This only works cross-application, not cross-origin. It has to come from an ODT. Or PDF - or anything else where I can inject nasty font-names.


Expected results:

No XSS. The sanitizer should detect and mitigate it just as it does for cross-origin copy&paste.

Note: The attached ODT works fine with copy&paste on Linux (Ubuntu, Gnome). I have one that works for Windows 7 as well. please let me know if you need both or if that one is good enough to repro.

Comment 1

[:Gijs (he/him)]

Ehsan, do you have ideas about how to fix this and/or who to ask instead? :-)

Comment 2

[Matt Wobensmith [:mwobensmith][:matt:]]

Definitely incorrect, requires user interaction/social engineering, rating sec-low.

Comment 3

[(no longer active)]

(In reply to :Gijs Kruitbosch from comment #1)
> Ehsan, do you have ideas about how to fix this and/or who to ask instead? :-)

Yes, we have some sanitization code to catch these cases, but IIRC it doesn't handle anything from SVG.

Comment 4

[(no longer active)]

Mario, can you please test this against other browsers as well?  When we first fixed this issue over in bug 520189, other browsers were similarly vulnerable as well.

Comment 5

[(no longer active)]

(In reply to Matt Wobensmith from comment #2)
> Definitely incorrect, requires user interaction/social engineering, rating
> sec-low.

Bug 520189 was considered sg:moderate.  Should this be reclassified?

Comment 6

[Frederik Braun [:freddy]]

I talked with Mario out of band before he filed this. Other browsers *are* similarly vulnerable.

Comment 7

[Mario Heiderich]

(In reply to :Ehsan Akhgari [Away: 1/29-2/20] (not reading bugmail, needinfo? me!) from comment #4)
> Mario, can you please test this against other browsers as well?  When we
> first fixed this issue over in bug 520189, other browsers were similarly
> vulnerable as well.

I did already and reports are out to all affected vendors (Google, MS). 
Chrome team is working on a fix, MSRC dialogue was initiated today.

Comment 8

[Matt Wobensmith [:mwobensmith][:matt:]]

RE: rating change. Sounds appropriate. Thanks.

Comment 9

[(no longer active)]

Mario, do I need to do something special to reproduce this?  When I paste what I copied from the latest version of OpenOffice.org into http://www-archive.mozilla.org/editor/midasdemo/, I just get "Hello!" and no black rectangle/script running.

Comment 10

[(no longer active)]

Henri, one way to fix this would be to disallow the SVG animate element altogether, since all the ways it can interact with the host document are hard to sanitize (for example, in the snippet in comment 0, if there was a dur attribute, the @to attribute would matter as well...)

Do you think that is a good fix?

Comment 11

[Mario Heiderich]

(In reply to :Ehsan Akhgari [Away: 1/29-2/20] (not reading bugmail, needinfo? me!) from comment #9)
> Mario, do I need to do something special to reproduce this?  When I paste
> what I copied from the latest version of OpenOffice.org into
> http://www-archive.mozilla.org/editor/midasdemo/, I just get "Hello!" and no
> black rectangle/script running.

I presume that this particular RTE uses designMode=on or document.execCommand('EditMode')? 
For these only Chrome and IE are affected.

It does repro cleanly in e.g. the Gmail webinterface (compose window). 
And yes, encouraging to test there sounds like a weird proposal ;) 

It also repros on the test-website I linked above.

Comment 12

[(no longer active)]

(In reply to Mario Heiderich from comment #11)
> (In reply to :Ehsan Akhgari [Away: 1/29-2/20] (not reading bugmail,
> needinfo? me!) from comment #9)
> > Mario, do I need to do something special to reproduce this?  When I paste
> > what I copied from the latest version of OpenOffice.org into
> > http://www-archive.mozilla.org/editor/midasdemo/, I just get "Hello!" and no
> > black rectangle/script running.
> 
> I presume that this particular RTE uses designMode=on or
> document.execCommand('EditMode')? 

Yes.

> For these only Chrome and IE are affected.

Huh, yeah, that's because we disable script execution for designMode document.  I should have known better. :-)

> It does repro cleanly in e.g. the Gmail webinterface (compose window). 
> And yes, encouraging to test there sounds like a weird proposal ;) 
> 
> It also repros on the test-website I linked above.

Do you mean html5sec.org/copypaste?  I tried the "3rd" box there, and it didn't work...

(Not that it matters much, I think I know what's going on here, just curious to know what prevents me from being able to reproduce.)

Comment 13

[Mario Heiderich]

(In reply to :Ehsan Akhgari [Away: 1/29-2/20] (not reading bugmail, needinfo? me!) from comment #10)
> Henri, one way to fix this would be to disallow the SVG animate element
> altogether, since all the ways it can interact with the host document are
> hard to sanitize (for example, in the snippet in comment 0, if there was a
> dur attribute, the @to attribute would matter as well...)
> 
> Do you think that is a good fix?

Then you also might consider to prohibit the SET element. Does similar things. If it's not prohibited already.

> Do you mean html5sec.org/copypaste?  I tried the "3rd" box there, and it didn't work...

Yes. Works for me on Ubuntu 12, Firefox 35.0 and Windows 7, FF 35.0 as well. I didn't test thoroughly on OSX yet. Simply having a data:text/html,<divcontenteditable>here</div> should also do the job.

Comment 14

[(no longer active)]

(In reply to Mario Heiderich from comment #13)
> (In reply to :Ehsan Akhgari [Away: 1/29-2/20] (not reading bugmail,
> needinfo? me!) from comment #10)
> > Henri, one way to fix this would be to disallow the SVG animate element
> > altogether, since all the ways it can interact with the host document are
> > hard to sanitize (for example, in the snippet in comment 0, if there was a
> > dur attribute, the @to attribute would matter as well...)
> > 
> > Do you think that is a good fix?
> 
> Then you also might consider to prohibit the SET element. Does similar
> things. If it's not prohibited already.

Agreed.

> > Do you mean html5sec.org/copypaste?  I tried the "3rd" box there, and it didn't work...
> 
> Yes. Works for me on Ubuntu 12, Firefox 35.0 and Windows 7, FF 35.0 as well.
> I didn't test thoroughly on OSX yet. Simply having a
> data:text/html,<divcontenteditable>here</div> should also do the job.

Huh, I just tested on Firefox 35, and the test case works there, but not on Nightly, which seems to suggest that something that has landed since Firefox 35 might have fixed this.

Anthony, Tracy, any chance you could do a bisection to see what fixed this?  Notice that this is kind of the reverse of a normal bisection, in that we are looking for a changeset that fixed this.  Thanks!

Comment 15

[u279076]

(In reply to [Away: 1/29-2/20] from comment #14)
> Anthony, Tracy, any chance you could do a bisection to see what fixed this? 
> Notice that this is kind of the reverse of a normal bisection, in that we
> are looking for a changeset that fixed this.  Thanks!

I'll take a look to see if I can reproduce this first. Assuming I can then I'll go ahead and attempt a bisection for the fix.

Comment 16

[u279076]

(In reply to Anthony Hughes, QA Mentor (:ashughes) from comment #15)
> I'll take a look to see if I can reproduce this first. Assuming I can then
> I'll go ahead and attempt a bisection for the fix.

I do confirm that this reproduces for me on Ubuntu 14.04.1 with Firefox 35.0 but does not with Firefox 38.0a1 2015-01-28. I'll start trying to narrow the fix range down.

Comment 17

[u279076]

mozilla-central
===============
Broke: https://hg.mozilla.org/mozilla-central/rev/0b81c10a9074 [Firefox 36.0a1 2014-11-03]
Fixed: https://hg.mozilla.org/mozilla-central/rev/b2b10231606b [Firefox 36.0a1 2014-11-04]
Pushlog: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=0b81c10a9074&tochange=b2b10231606b

mozilla-inbound
===============
Broke: https://hg.mozilla.org/integration/mozilla-inbound/rev/8b87594bb95e
Fixed: https://hg.mozilla.org/integration/mozilla-inbound/rev/f129b17f9067
Pushlog: https://hg.mozilla.org/integration/mozilla-inbound//pushloghtml?fromchange=8b87594bb95e&tochange=f129b17f9067

There's a lot of changes here. I might need some help trying to bisect this further. Any suggestions?

Comment 18

[u279076]

What's interesting here is that this must have broke again at some point. Based on the above pushlog, this should be fixed in Firefox 36 but I can still reproduce this in the latest Firefox 36 Beta build.

Comment 19

[(no longer active)]

Please perform more detailed bisection by building.  If there are multiple ranges where this was fixed then regressed then fixed later, please focus on the latest fix, as that is probably more telling as to why this can't be reproduced on trunk.  Thank you!

Comment 20

[u279076]

I checked this some more. It appears this was "fixed" for Firefox 36.0a1 on November 4 but broke again on November 24. The relevant information is below.

Most recent fix range:
> Broke in Firefox 36.0a1 2014-11-03: https://hg.mozilla.org/mozilla-central/rev/0b81c10a9074
> Fixed in Firefox 36.0a1 2014-11-04: https://hg.mozilla.org/mozilla-central/rev/b2b10231606b
> Pushlog: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=0b81c10a9074&tochange=b2b10231606b

Most recent regression range:
> Fixed in Firefox 36.0a1 2014-11-23: https://hg.mozilla.org/mozilla-central/rev/c9cfa9b91dea
> Broke in Firefox 36.0a1 2014-11-24: https://hg.mozilla.org/mozilla-central/rev/8c02f3280d0c
> Pushlog: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=c9cfa9b91dea&tochange=8c02f3280d0c

I noticed that in the most recent regression range it seems the only change is prefing off e10s (bug 1101193). Investigating that further I found that disabling e10s in the "fixed" build on 2014-11-23 reproduced this bug. As a result I'll have to re-investigate the range on Nightly with e10s off.

Comment 21

[u279076]

So I just retested this in the latest Nightly with e10s disabled and the bug reproduces, so in fact this bug is *not* fixed.

Comment 22

[(no longer active)]

Oh, something is broken with e10s then...  That's kind of worrying.  Not sure how we'd want to spin that part off to a (public?) bug, and I'm leaving for vacation in a few mins.  Needinfoing Bill for the e10s specific part.

Comment 23

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Yes, pasting of non-text/plain content is bug 1071562.

Comment 24

[Henri Sivonen (:hsivonen)]

(In reply to [Away: 1/29-2/20] from comment #3)
> (In reply to :Gijs Kruitbosch from comment #1)
> > Ehsan, do you have ideas about how to fix this and/or who to ask instead? :-)
> 
> Yes, we have some sanitization code to catch these cases, but IIRC it
> doesn't handle anything from SVG.

This is incorrect. Our sanitizer does know about SVG. The problem is that the sanitizer treats declarative animations as safe. Which they apparently aren't!

(In reply to [Away: 1/29-2/20] from comment #10)
> Henri, one way to fix this would be to disallow the SVG animate element
> altogether, since all the ways it can interact with the host document are
> hard to sanitize (for example, in the snippet in comment 0, if there was a
> dur attribute, the @to attribute would matter as well...)
> 
> Do you think that is a good fix?

It's a bit sad if we have to remove all declarative animations because of this. It would be nicer to be able to sanitize conditionally based on the the attributeName attribute of <animate> or <set>. Do you think that would be too risky?

Specifically, drop SVG <animate> or SVG <set> if either of the following is true:
 1) The parent is not in the SVG namespace.
 2) The whitespace-trimmed value of the attributeName attribute is not in kAttributesSVG (https://dxr.mozilla.org/mozilla-central/source/dom/base/nsTreeSanitizer.cpp#367).

Comment 25

[:Gijs (he/him)]

This was fixed by bug 1182778 (on 42+), and bug 1203102 covers relaxing the restrictions here as suggested in comment #24.

Comment 26

[Bogdan Maris, Desktop QA]

Reproduced the initial issue using the odt file from comment 0 on old Nightly from 2015-01-27 and verified that the issue is fixed using Firefox 42.0RC, latest Developer Edition 43.0a2 and latest Nightly 44.0a1 across platforms (Ubuntu 14.04 64-bit and Mac OS X 10.11).