455472 - (CVE-2010-0162) [FIX]code injection with Content-Type: application/octet-stream, embed and svg - no plugins required

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[georgi - hopefully not receiving bugspam]

Attachment: svg.bin - ct binary on purpose

ability to upload file with  Content-Type: application/octet-stream to a web server means ability to execute javascript from the same server.

done via:
<embed id="s1" type="image/svg+xml" src="http://sarwar/svgim1.bin">

svgim1.bin is svg containing javascript

no plugins are required

Comment 1

[georgi - hopefully not receiving bugspam]

Attachment: testcase getsvg2.html

Comment 2

[georgi - hopefully not receiving bugspam]

Attachment: correct testcase

Comment 3

[Boris Zbarsky [:bzbarsky]]

Is this behavior interoperable across browsers?  I would think it is.

Comment 4

[georgi - hopefully not receiving bugspam]

> Is this behavior interoperable across browsers?  I would think it is.

the testcase is not buggy in konqueror 3.5.9 - no alert. though i am not sure this version of konq. understands svg at all.

interesting in this bug is that the browser plays something like a plugin if i understand the meaning of embed correctly.

Comment 5

[georgi - hopefully not receiving bugspam]

on konqueror 3.5.9

<embed src="http://server/index.bin" type="text/html"></embed>

displays the embedded HTML. this testcase fails on firefox (missing plugin).

will try safari soon.

bz: note that i am not writing about plugins on purpose.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Safari seems to save the file.  Opera puts up the alert.  I don't have IE on hand.  Note that the application/octet-stream special-casing here was added to fix bug 389677.

Comment 7

[Boris Zbarsky [:bzbarsky]]

Oh, and we don't allow subdocuments in embed, except for SVG.

Comment 8

[Boris Zbarsky [:bzbarsky]]

And note also that this behavior is the same for <object> as for <embed>.

Comment 9

[georgi - hopefully not receiving bugspam]

confirming that safari prompts for saving the svg, same for html with index.bin

Comment 10

[georgi - hopefully not receiving bugspam]

>Oh, and we don't allow subdocuments in embed, except for SVG.

cross site i can get the document via getSVGDocument() but can't get .firstChild

Comment 11

[Boris Zbarsky [:bzbarsky]]

> cross site i can get the document via getSVGDocument()

Hmm.  That might actually be a bug; not sure about that.  Blake, please file it as needed if it is?  I'd think we would disallow this, since it's not getting an XOW.

georgi, is the not being able to get firstChild also true on trunk, or just with Firefox 3?

Comment 12

[georgi - hopefully not receiving bugspam]

bz: i am trying to steal the xml cross site and get security exception. on same site i get .firstChild fine. i don't see any bug, except probably even getting the document via getSVGDocument()

Comment 13

[georgi - hopefully not receiving bugspam]

> georgi, is the not being able to get firstChild also true on trunk, or just
> with Firefox 3?

to clarify: when svg is served from the same site, i CAN GET .firstChild on both trunk and 1.9 branch.

i fail when it is cross site - this seems reasonable

Comment 14

[Boris Zbarsky [:bzbarsky]]

I don't think you should be able to getSVGDocument() cross-site.  And I'm afraid that if you _can_ do it you might be able to get firstChild on trunk (if not now, then as we remove more redundany security checks).

Comment 15

[georgi - hopefully not receiving bugspam]

> I don't think you should be able to getSVGDocument() cross-site.

i definitely can get cross site |getSVGDocument()| on both trunk and 1.9 branch.
.firstChild throws cross site

Comment 16

[georgi - hopefully not receiving bugspam]

Attachment: demo for cross site getSVGDocument()

Comment 17

[Lucas Adamski [:ladamski]]

There seems to be a little confusion around this bug.  Cross-site means that one site can script into another site, which is not quite the same thing as uploading a file to a server that then results in code execution.

Comment 18

[Boris Zbarsky [:bzbarsky]]

The point georgi is making is that this allows you to inject code to run with the given server's permissions, from whence it can script other pages on that server.

Comment 19

[Lucas Adamski [:ladamski]]

To clarify, I agree that this can result in malicious code injection, but it is not clear to me whether that's the only issue or if we also have an issue with cross-domain code injection via DOM navigation.

Comment 20

[Boris Zbarsky [:bzbarsky]]

DOM navigation of what?

Comment 21

[Blake Kaplan (:mrbkap) (inactive)]

(In reply to comment #14)
> I don't think you should be able to getSVGDocument() cross-site.  And I'm
> afraid that if you _can_ do it you might be able to get firstChild on trunk (if
> not now, then as we remove more redundany security checks).

We should split this into a new bug. Either getSVGDocument needs to do a security check or we need to wrap object elements in XOWs, even when same-origin (like we do for HTML[I]FrameElements.

Comment 22

[Boris Zbarsky [:bzbarsky]]

Filed bug 455629.

Comment 23

[georgi - hopefully not receiving bugspam]

ok, the description of this bug is flamable.

looks like bz got the idea of potential misuse and code injection.

Comment 24

[georgi - hopefully not receiving bugspam]

since the validity of this bug is questioned i won't reopen it if someone invalidates it.

>Oh, and we don't allow subdocuments in embed, except for SVG.

if this is true wouldn't treating svg content type specially be a nice ugly kludge?

Comment 25

[georgi - hopefully not receiving bugspam]

>if this is true wouldn't treating svg content type specially be a nice ugly
>kludge?

this seems dumb. <object> has same trouble with at least text/html

Comment 26

[Boris Zbarsky [:bzbarsky]]

Right.  That's what I said in comment 8.

Comment 27

[Jesse Ruderman]

Given the publicity around a similar issue with Flash, I find it hard to believe this will remain secret for long.

http://news.slashdot.org/story/09/11/12/2337236/Flash-Vulnerability-Found-Adobe-Says-No-Fix-Forthcoming?art_pos=1

Comment 28

[Jesse Ruderman]

If I understand correctly, this is "[sg:high] against any site that allows uploads".

Comment 29

[Boris Zbarsky [:bzbarsky]]

We could probably avoid dispatching eType_Document if the type was application/octet-stream or text/plain on the channel...  Would that help?

Comment 30

[Jesse Ruderman]

Not really.  It needs to be whitelist-based, since many sites allow anything to be uploaded as image/jpeg.

Comment 31

[Boris Zbarsky [:bzbarsky]]

Uh... if it's uploaded as image/jpeg, we're not going to render it as SVG, so there's no problem.  Or am I missing something?

Comment 32

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I don't think this should block, since it's in 1.9.1 and earlier. But it does seem to me we should do comment #29 ASAP. Boris, do you want to take this?

Comment 33

[Boris Zbarsky [:bzbarsky]]

Attachment: Proposed fix

I assume that landing the testcase with the fix is not a problem, but if it is please tell me so!

Comment 34

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 413073 [details] [diff] [review]
Proposed fix

We might want to take this for 1.9.2...  I think this should be fairly safe web-compat wise.

Comment 35

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 413073 [details] [diff] [review]
Proposed fix

approval1.9.2 requests aren't currently being monitored, since we're nearing RC freeze and there are too many outstanding requests, so I'm clearing this request. Feel free to re-request approval if you are confident that it's worth drivers' time to consider whether this non-blocker needs to land for 1.9.2 at this stage.

Comment 36

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 413073 [details] [diff] [review]
Proposed fix

a192=beltzner

Need to keep an eye out for web compat issues, but need to get this in for rc

Comment 37

[Boris Zbarsky [:bzbarsky]]

Pushed http://hg.mozilla.org/mozilla-central/rev/71b14537359b

Comment 38

[Boris Zbarsky [:bzbarsky]]

Attachment: 1.9.2 branch merge

Comment 39

[Boris Zbarsky [:bzbarsky]]

Pushed http://hg.mozilla.org/releases/mozilla-1.9.2/rev/c76bdccb63c9

Comment 40

[Boris Zbarsky [:bzbarsky]]

Attachment: 1.9.1 branch merge

Comment 41

[Boris Zbarsky [:bzbarsky]]

Attachment: 1.9.0 merge

Comment 42

[Daniel Veditz [:dveditz]]

Comment on attachment 415913 [details] [diff] [review]
1.9.0 merge

Approved for 1.9.1.7 and 1.9.0.17, a=dveditz for release-drivers

Comment 43

[Boris Zbarsky [:bzbarsky]]

Pushed http://hg.mozilla.org/releases/mozilla-1.9.1/rev/2e73012d6766

Checked into CVS:

Checking in content/base/test/Makefile.in;
/cvsroot/mozilla/content/base/test/Makefile.in,v  <--  Makefile.in
new revision: 1.82; previous revision: 1.81
done
RCS file: /cvsroot/mozilla/content/base/test/test_bug455472.html,v
done
Checking in content/base/test/test_bug455472.html;
/cvsroot/mozilla/content/base/test/test_bug455472.html,v  <--  test_bug455472.html
initial revision: 1.1
done
Checking in content/base/src/nsObjectLoadingContent.cpp;
/cvsroot/mozilla/content/base/src/nsObjectLoadingContent.cpp,v  <--  nsObjectLoadingContent.cpp
new revision: 1.101; previous revision: 1.100

Comment 44

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Boris, I have one question before verifying this fix. The 3rd testcase (demo for cross site getSVGDocument) should still fail? Or are both alert boxes expected?

Comment 45

[Boris Zbarsky [:bzbarsky]]

The 3rd testcase should have two alerts; the first should say "doc=[object SVGDocument]" and the second should have "exception=....."

Comment 46

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Verified fixed on 1.9.1 with builds on all platforms by running the above three testcases.

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.8pre) Gecko/20100128 Shiretoko/3.5.8pre (.NET CLR 3.5.30729) ID:20100128044603

Comment 47

[Al Billings [:abillings - ex-MoCo]]

Verified for 1.9.0.18 with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.16pre) Gecko/2010020215 GranParadiso/3.0.18pre (.NET CLR 3.5.30729) using test cases.