Last Comment Bug 368763 - (CVE-2007-0994) Arbitrary code execution by using javascript: url with <img> tag
: Arbitrary code execution by using javascript: url with <img> tag
[sg:critical] hold for tbird release
: regression, verified1.8.0.10, verified1.8.1.2
Product: Core
Classification: Components
Component: Security (show other bugs)
: 1.8 Branch
: x86 Windows XP
: -- normal (vote)
: ---
Assigned To: Brian Crowder
: David Keeler [:keeler] (use needinfo?)
Depends on:
Blocks: 351370
  Show dependency treegraph
Reported: 2007-01-30 16:14 PST by moz_bug_r_a4
Modified: 2007-08-06 18:44 PDT (History)
14 users (show)
dveditz: blocking1.8.1.2+
dveditz: blocking1.8.0.10+
bzbarsky: in‑testsuite?
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Branch port of bug 363594 (2.30 KB, patch)
2007-02-05 13:27 PST, Boris Zbarsky [:bz] (still a bit busy)
dveditz: review+
jaymoz: approval1.8.1.2+
jaymoz: approval1.8.0.10+
Details | Diff | Splinter Review
screenshot (158.45 KB, image/jpeg)
2007-03-05 22:03 PST, moz_bug_r_a4
no flags Details

Description moz_bug_r_a4 2007-01-30 16:14:01 PST
Created attachment 253418 [details]

This is very similar to bug 363594.  This affects only 1.8/1.8.0 branches.

When javascript: url is set by script, the access checks work properly.

  <img id="i">
  i.src = "javascript:...";

But, when javascript: url is set by <img> (or <link>, <style>) tag, the access
checks don't work properly.

  <img src="javascript:...">

Thus, sandboxed script can access xbl.method's clone parent and xbl compilation
scope to run arbitrary code with chrome privileges.

Disabling JavaScript does *not* block this exploit.  Because, javascript: url
can be evaluated even when JavaScript is disabled. (this issue seems to be
fixed in bug 368655, according to a CVS Log.)
Comment 1 Brian Crowder 2007-01-31 14:39:48 PST
Boris:  Have you had a chance to look at this yet?  Do you want me to take it?  If so, can you give me a hint where to start looking (if this works along the same lines as the other bug, I assume it is in browser code, not spidermonkey)?
Comment 2 Boris Zbarsky [:bz] (still a bit busy) 2007-01-31 16:46:44 PST
I didn't even realize this bug existed -- none of the changes made on it so far are things I get bugmail for.  So I haven't looked, no.

As for where to start looking... At this point, the javascript: URI on branch is run in a sandbox with a made-up principal.  So I'd suspect the bug is elsewhere.

I'd probably start with a breakpoint in EvalInSandboxObject, when I get there set a breakpoint in nsScriptSecurityManager::CheckPropertyAccessImpl, wait until the property in question comes through and see what's going on....  Of course if that property never comes through CheckPropertyAccessImpl that'll narrow down things right there.  ;)
Comment 3 moz_bug_r_a4 2007-02-01 10:09:52 PST
Created attachment 253638 [details]
testcase 2 - reduced testcase

This does not work when JavaScript is disabled.  And, this is almost the same
as the testcase in bug 363594.
Comment 4 Daniel Veditz [:dveditz] 2007-02-01 14:20:26 PST
FF2.0/ are fine, FF2.0.0.1 is affected (I'll assume as well for now, but will check in a bit).

-> crowder based purely on enthusiasm, but maybe should be part of Boris's fix for bug 363594.
Comment 5 Brian Crowder 2007-02-05 10:56:32 PST
I'm not having a lot of luck with this, it's in code that I have not looked at much before, if at all.  Might be better owned by bz or someone else?
Comment 6 Brian Crowder 2007-02-05 12:28:14 PST
Adding mrbkap and Brendan.  If we want this done today, I am going to need help.
Comment 7 Boris Zbarsky [:bz] (still a bit busy) 2007-02-05 13:12:00 PST
So finally had time to sit down and read this bug and more importantly the testcase.  Sounds to me like we just need to land the patch for bug 363594 on the branches, no?

Unfortunately, it's been a while since I built branch, so I won't be able to test for a bit.  But I'll post a patch if someone is willing to test.
Comment 8 Brian Crowder 2007-02-05 13:14:43 PST
I'll gladly test a patch.
Comment 9 Boris Zbarsky [:bz] (still a bit busy) 2007-02-05 13:25:33 PST
Actually, it's not quite so simple, and I'm not as convinced that the fix for bug 363594 is correct.

The reason bug 363594 is trunk-only is that in that case we were evaluating the JS async, so the caller cx was no longer on the context stack.

All well and good, but fixing that by pushing the _target_ cx on the context stack (as we did) only works as long as the target is not itself privileged in some way.  If push comes to shove, we should just land the patch I'll attach (port of bug 363594 to branches) because it's better than nothing, but it's not perfect.

Ideally, we would push on the stack the same JSContext that the sandbox used for evaluation.  Failing that, we should probably create a vanilla context with the sandbox object as its global, etc, just like EvalInSandboxObject() ends up doing.
Comment 10 Boris Zbarsky [:bz] (still a bit busy) 2007-02-05 13:27:40 PST
Created attachment 254063 [details] [diff] [review]
Branch port of bug 363594

This will fix this specific testcase.... At least with this, getting UniversalXPConnect involves getting your hands on a window whose global has UniversalXPConnect and then doing the load in some object connected with that window....
Comment 11 Boris Zbarsky [:bz] (still a bit busy) 2007-02-05 13:41:47 PST
So I considered just doing a JS_NewContext here.  But then we'd either need a bunch of code to make sure that bug 368714 stays fixed or we'd allow infinite loops in toString/valueOf to hang the app.  :(

I suppose we could more or less copy/paste the ContextHolder class into this code...
Comment 12 Brian Crowder 2007-02-05 14:31:36 PST
With this patch applied, neither of these testcases yields an alert window for me.
Comment 13 Brian Crowder 2007-02-05 14:31:54 PST
(that was testing with the 1.8 branch, btw)
Comment 14 Brian Crowder 2007-02-05 14:45:43 PST
Applied and built on 1.8.0 cleanly.  If we need this fixed today, I recommend we land Boris' patch.
Comment 15 Brian Crowder 2007-02-05 15:11:55 PST
Comment on attachment 254063 [details] [diff] [review]
Branch port of bug 363594

Nominating for branches since I can't find bz at the moment.
Comment 16 Brian Crowder 2007-02-05 15:13:11 PST
Comment on attachment 254063 [details] [diff] [review]
Branch port of bug 363594

Who's a good sr?
Comment 17 Daniel Veditz [:dveditz] 2007-02-05 16:44:04 PST
Comment on attachment 254063 [details] [diff] [review]
Branch port of bug 363594

Comment 18 Jay Patel [:jay] 2007-02-05 16:45:15 PST
Comment on attachment 254063 [details] [diff] [review]
Branch port of bug 363594

Approved for both branches, a=jay (Thanks for the quick turnaround on the patch bz/crowder!)
Comment 19 Brian Crowder 2007-02-05 17:09:08 PST
Checking in dom/src/jsurl/nsJSProtocolHandler.cpp;
/cvsroot/mozilla/dom/src/jsurl/nsJSProtocolHandler.cpp,v  <--  nsJSProtocolHandler.cpp
new revision:; previous revision:

brian-crowders-computer:~/mozcvs/mozilla crowder$ cvs ci dom/src/jsurl/nsJSProtocolHandler.cppChecking in dom/src/jsurl/nsJSProtocolHandler.cpp;
/cvsroot/mozilla/dom/src/jsurl/nsJSProtocolHandler.cpp,v  <--  nsJSProtocolHandler.cpp
new revision:; previous revision:
Comment 20 Martijn Wargers [:mwargers] (not working for Mozilla) 2007-02-08 08:58:21 PST
Verified fixed on branches, with a 2007-01-31 build of the 1.8.1 branch, I see the alerts in the testcases.
With a 2007-02-07 build of the 1.8.1 branch, I don't get any alerts anymore, instead I get these error messages in the js console:
Error: uncaught exception: Permission denied to get property Function.__proto__
Error: uncaught exception: Permission denied to get property String.__parent__

Also verified fixed with the latest 1.8.0.x branch build.
Comment 21 Daniel Veditz [:dveditz] 2007-02-22 04:40:01 PST
regression between and Although fixed by the patch for trunk bug 363594, the blame for that regression was laid on trunk-only bug 351633. Would be nice to be able to identify the branch regressor.
Comment 22 Martijn Wargers [:mwargers] (not working for Mozilla) 2007-02-22 05:42:08 PST
On Firefox2, this seems to have regressed between 2006-12-01 and 2006-12-03:
There is a check-in by jst that looks like the likely cause, it doesn't have a bug number associated with it, but I think it's bug 351370.
So I think the branch patch for bug 351370 was the branch regressor.
Comment 23 Boris Zbarsky [:bz] (still a bit busy) 2007-02-22 08:56:08 PST
Oh, yeah.  Absolutely.

That said, did anyone file a bug to fix this correctly?  See comment 9.  The bandaid we checked in is sort of ok, but we should really fix this correctly.
Comment 24 Daniel Veditz [:dveditz] 2007-02-22 22:17:38 PST
Because of bug 368655 this one affects Thunderbird and SeaMonkey mail even with JavaScript disabled by default. This could be used to construct an email worm. Do not un-hide this bug until Thunderbird has been released for at least a week or so.
Comment 26 Boris Zbarsky [:bz] (still a bit busy) 2007-02-27 21:28:02 PST
I filed bug 372075 on comment 9.  I think we should fix it for the next security release, but I'm not sure I'll be able to work on it...  :(
Comment 27 Daniel Veditz [:dveditz] 2007-03-05 11:59:02 PST
(In reply to comment #24)
> Because of bug 368655 this one affects Thunderbird and SeaMonkey mail even with
> JavaScript disabled by default.

I was wrong: Thunderbird will not run a javascript: url in an img.src whether javascript is enabled or not. I suspected the remote content filter (since that whitelists only http(s) and ftp) but they're not run even if you load remote images.

I have not tested SeaMonkey mail.
Comment 28 moz_bug_r_a4 2007-03-05 22:01:22 PST
By the way, is this a bugzilla's bug?

I couldn't see comment #24 until after comment #27 has been added.  And now I
cannot see comment #25.
Comment 29 moz_bug_r_a4 2007-03-05 22:03:02 PST
Created attachment 257478 [details]
Comment 30 Boris Zbarsky [:bz] (still a bit busy) 2007-03-05 22:40:48 PST
Does bugzilla keep track of private comment flags?  I see nothing about them in the bug history, but comment 24 used to be private.  Now comment 25 is.  If someone twiddled the flags, that would make sense...
Comment 31 chris hofmann 2007-04-24 15:29:45 PDT
pvnick is doing a bit of research on XSS and also gathering up bugs with security related test cases to help add to the regression/certification test suites.  adding him to the cc list in these...

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