The default bug view has changed. See this FAQ.
Bug 368763 (CVE-2007-0994)

Arbitrary code execution by using javascript: url with <img> tag

VERIFIED FIXED

Status

()

Core
Security
VERIFIED FIXED
10 years ago
10 years ago

People

(Reporter: moz_bug_r_a4, Assigned: Brian Crowder)

Tracking

({regression, verified1.8.0.10, verified1.8.1.2})

1.8 Branch
x86
Windows XP
regression, verified1.8.0.10, verified1.8.1.2
Points:
---
Bug Flags:
blocking1.8.1.2 +
blocking1.8.0.10 +
in-testsuite ?

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sg:critical] hold for tbird release)

Attachments

(2 attachments)

(Reporter)

Description

10 years ago
Created attachment 253418 [details]
testcase

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.

Note:
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.)
Flags: blocking1.8.1.2+
Flags: blocking1.8.0.10+
Whiteboard: [sg:critical]
(Assignee)

Comment 1

10 years ago
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)?
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.  ;)
(Reporter)

Comment 3

10 years ago
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.
FF2.0/1.5.0.8 are fine, FF2.0.0.1 is affected (I'll assume 1.5.0.9 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.
Assignee: dveditz → crowder
Keywords: regression
Whiteboard: [sg:critical] → [sg:critical] needs patch
(Assignee)

Comment 5

10 years ago
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?
(Assignee)

Comment 6

10 years ago
Adding mrbkap and Brendan.  If we want this done today, I am going to need help.
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.
(Assignee)

Comment 8

10 years ago
I'll gladly test a patch.
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.
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....
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...
(Assignee)

Comment 12

10 years ago
With this patch applied, neither of these testcases yields an alert window for me.
(Assignee)

Comment 13

10 years ago
(that was testing with the 1.8 branch, btw)
(Assignee)

Comment 14

10 years ago
Applied and built on 1.8.0 cleanly.  If we need this fixed today, I recommend we land Boris' patch.
(Assignee)

Comment 15

10 years ago
Comment on attachment 254063 [details] [diff] [review]
Branch port of bug 363594

Nominating for branches since I can't find bz at the moment.
Attachment #254063 - Flags: approval1.8.1.2?
Attachment #254063 - Flags: approval1.8.0.10?
(Assignee)

Comment 16

10 years ago
Comment on attachment 254063 [details] [diff] [review]
Branch port of bug 363594

Who's a good sr?
Attachment #254063 - Flags: review?(jst)
Comment on attachment 254063 [details] [diff] [review]
Branch port of bug 363594

sr=dveditz
Attachment #254063 - Flags: review?(jst) → review+

Comment 18

10 years ago
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!)
Attachment #254063 - Flags: approval1.8.1.2?
Attachment #254063 - Flags: approval1.8.1.2+
Attachment #254063 - Flags: approval1.8.0.10?
Attachment #254063 - Flags: approval1.8.0.10+

Updated

10 years ago
Whiteboard: [sg:critical] needs patch → [sg:critical] needs landing
(Assignee)

Comment 19

10 years ago
Checking in dom/src/jsurl/nsJSProtocolHandler.cpp;
/cvsroot/mozilla/dom/src/jsurl/nsJSProtocolHandler.cpp,v  <--  nsJSProtocolHandler.cpp
new revision: 1.113.2.9; previous revision: 1.113.2.8
done

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: 1.113.2.2.2.4; previous revision: 1.113.2.2.2.3
done
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Keywords: regression → fixed1.8.0.10, fixed1.8.1.2
Resolution: --- → FIXED
Whiteboard: [sg:critical] needs landing → [sg:critical]
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.
Status: RESOLVED → VERIFIED
Keywords: fixed1.8.0.10, fixed1.8.1.2 → verified1.8.0.10, verified1.8.1.2
regression between 1.5.0.8/2.0 and 1.5.0.9/2.0.0.1. 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.
Keywords: regression
On Firefox2, this seems to have regressed between 2006-12-01 and 2006-12-03:
http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=MOZILLA_1_8_BRANCH&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2006-12-01+03&maxdate=2006-12-03+06&cvsroot=%2Fcvsroot
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.
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.
Blocks: 351370
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 1.5.0.10 has been released for at least a week or so.
Whiteboard: [sg:critical] → [sg:critical] hold for tbird release
Alias: CVE-2007-0994
Flags: in-testsuite?
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...  :(
(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.
(Reporter)

Comment 28

10 years ago
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.
(Reporter)

Comment 29

10 years ago
Created attachment 257478 [details]
screenshot
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

10 years ago
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...
Group: security
You need to log in before you can comment on or make changes to this bug.