Last Comment Bug 455472 - (CVE-2010-0162) [FIX]code injection with Content-Type: application/octet-stream, embed and svg - no plugins required
(CVE-2010-0162)
: [FIX]code injection with Content-Type: application/octet-stream, embed and sv...
Status: RESOLVED FIXED
[sg:moderate]
: testcase, verified1.9.0.18, verified1.9.1
Product: Core
Classification: Components
Component: DOM (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla1.9.3a1
Assigned To: Boris Zbarsky [:bz] (still a bit busy)
:
: Andrew Overholt [:overholt]
Mentors:
Depends on: 455629 548290
Blocks:
  Show dependency treegraph
 
Reported: 2008-09-16 01:59 PDT by georgi - hopefully not receiving bugspam
Modified: 2010-06-20 09:56 PDT (History)
15 users (show)
roc: blocking1.9.2-
dveditz: blocking1.9.0.18+
dveditz: wanted1.9.0.x+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
beta5-fixed
.8+
.8-fixed


Attachments
svg.bin - ct binary on purpose (251 bytes, application/octet-stream)
2008-09-16 01:59 PDT, georgi - hopefully not receiving bugspam
no flags Details
testcase getsvg2.html (199 bytes, text/html)
2008-09-16 02:00 PDT, georgi - hopefully not receiving bugspam
no flags Details
correct testcase (218 bytes, text/html)
2008-09-16 02:02 PDT, georgi - hopefully not receiving bugspam
no flags Details
demo for cross site getSVGDocument() (483 bytes, text/html)
2008-09-16 08:03 PDT, georgi - hopefully not receiving bugspam
no flags Details
Proposed fix (3.98 KB, patch)
2009-11-18 07:37 PST, Boris Zbarsky [:bz] (still a bit busy)
jst: review+
mbeltzner: approval1.9.2+
Details | Diff | Splinter Review
1.9.2 branch merge (4.14 KB, patch)
2009-12-03 10:55 PST, Boris Zbarsky [:bz] (still a bit busy)
no flags Details | Diff | Splinter Review
1.9.1 branch merge (3.87 KB, patch)
2009-12-03 10:58 PST, Boris Zbarsky [:bz] (still a bit busy)
dveditz: approval1.9.1.8+
Details | Diff | Splinter Review
1.9.0 merge (3.83 KB, patch)
2009-12-03 11:38 PST, Boris Zbarsky [:bz] (still a bit busy)
dveditz: approval1.9.0.18+
Details | Diff | Splinter Review

Description georgi - hopefully not receiving bugspam 2008-09-16 01:59:15 PDT
Created attachment 338822 [details]
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 2008-09-16 02:00:55 PDT
Created attachment 338824 [details]
testcase getsvg2.html
Comment 2 georgi - hopefully not receiving bugspam 2008-09-16 02:02:06 PDT
Created attachment 338825 [details]
correct testcase
Comment 3 Boris Zbarsky [:bz] (still a bit busy) 2008-09-16 05:33:33 PDT
Is this behavior interoperable across browsers?  I would think it is.
Comment 4 georgi - hopefully not receiving bugspam 2008-09-16 06:29:14 PDT
> 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 2008-09-16 06:45:22 PDT
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 [:bz] (still a bit busy) 2008-09-16 06:58:39 PDT
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 [:bz] (still a bit busy) 2008-09-16 06:58:58 PDT
Oh, and we don't allow subdocuments in embed, except for SVG.
Comment 8 Boris Zbarsky [:bz] (still a bit busy) 2008-09-16 06:59:18 PDT
And note also that this behavior is the same for <object> as for <embed>.
Comment 9 georgi - hopefully not receiving bugspam 2008-09-16 07:10:51 PDT
confirming that safari prompts for saving the svg, same for html with index.bin
Comment 10 georgi - hopefully not receiving bugspam 2008-09-16 07:13:14 PDT
>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 [:bz] (still a bit busy) 2008-09-16 07:21:34 PDT
> 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 2008-09-16 07:33:18 PDT
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 2008-09-16 07:37:14 PDT
> 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 [:bz] (still a bit busy) 2008-09-16 07:47:22 PDT
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 2008-09-16 07:52:41 PDT
> 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 2008-09-16 08:03:50 PDT
Created attachment 338868 [details]
demo for cross site getSVGDocument()
Comment 17 Lucas Adamski [:ladamski] 2008-09-16 11:17:12 PDT
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 [:bz] (still a bit busy) 2008-09-16 11:22:18 PDT
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] 2008-09-16 11:40:37 PDT
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 [:bz] (still a bit busy) 2008-09-16 12:24:18 PDT
DOM navigation of what?
Comment 21 Blake Kaplan (:mrbkap) 2008-09-16 18:05:48 PDT
(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 [:bz] (still a bit busy) 2008-09-16 18:24:01 PDT
Filed bug 455629.
Comment 23 georgi - hopefully not receiving bugspam 2008-09-17 00:46:55 PDT
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 2008-09-17 04:44:21 PDT
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 2008-09-17 04:48:28 PDT
>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 [:bz] (still a bit busy) 2008-09-17 05:52:42 PDT
Right.  That's what I said in comment 8.
Comment 27 Jesse Ruderman 2009-11-13 17:15:58 PST
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 2009-11-13 17:17:44 PST
If I understand correctly, this is "[sg:high] against any site that allows uploads".
Comment 29 Boris Zbarsky [:bz] (still a bit busy) 2009-11-13 18:27:59 PST
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 2009-11-13 20:33:10 PST
Not really.  It needs to be whitelist-based, since many sites allow anything to be uploaded as image/jpeg.
Comment 31 Boris Zbarsky [:bz] (still a bit busy) 2009-11-13 21:08:40 PST
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) 2009-11-16 19:13:11 PST
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 [:bz] (still a bit busy) 2009-11-18 07:37:11 PST
Created attachment 413073 [details] [diff] [review]
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 [:bz] (still a bit busy) 2009-11-18 18:09:44 PST
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: gavin@gavinsharp.com] 2009-12-01 17:44:32 PST
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] 2009-12-02 05:22:14 PST
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 [:bz] (still a bit busy) 2009-12-02 20:58:07 PST
Pushed http://hg.mozilla.org/mozilla-central/rev/71b14537359b
Comment 38 Boris Zbarsky [:bz] (still a bit busy) 2009-12-03 10:55:31 PST
Created attachment 415906 [details] [diff] [review]
1.9.2 branch merge
Comment 39 Boris Zbarsky [:bz] (still a bit busy) 2009-12-03 10:56:00 PST
Pushed http://hg.mozilla.org/releases/mozilla-1.9.2/rev/c76bdccb63c9
Comment 40 Boris Zbarsky [:bz] (still a bit busy) 2009-12-03 10:58:40 PST
Created attachment 415907 [details] [diff] [review]
1.9.1 branch merge
Comment 41 Boris Zbarsky [:bz] (still a bit busy) 2009-12-03 11:38:13 PST
Created attachment 415913 [details] [diff] [review]
1.9.0 merge
Comment 42 Daniel Veditz [:dveditz] 2009-12-14 14:23:18 PST
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 [:bz] (still a bit busy) 2009-12-15 18:13:57 PST
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) 2010-01-28 08:26:37 PST
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 [:bz] (still a bit busy) 2010-01-28 19:06:03 PST
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) 2010-01-29 06:10:50 PST
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] 2010-02-10 17:33:04 PST
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.

Note You need to log in before you can comment on or make changes to this bug.