Last Comment Bug 292589 - [FIX]XBL load missing content policy check (Thunderbird not blocking remote content)
: [FIX]XBL load missing content policy check (Thunderbird not blocking remote c...
Status: RESOLVED FIXED
[sg:fix] have patch
: fixed-aviary1.0.5, fixed1.7.9, privacy
Product: Core
Classification: Components
Component: XBL (show other bugs)
: 1.7 Branch
: x86 Windows XP
: P1 normal (vote)
: mozilla1.8beta2
Assigned To: Boris Zbarsky [:bz]
:
Mentors:
Depends on: 293778
Blocks:
  Show dependency treegraph
 
Reported: 2005-05-02 02:29 PDT by moz_bug_r_a4
Modified: 2007-04-01 15:00 PDT (History)
13 users (show)
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
Add content policy check to XBL (2.11 KB, patch)
2005-05-10 10:43 PDT, Boris Zbarsky [:bz]
jst: superreview+
shaver: approval‑aviary1.0.5+
dbaron: approval‑aviary1.1a1+
shaver: approval1.7.8+
Details | Diff | Splinter Review
1.7 branch fix (1.60 KB, patch)
2005-05-13 20:12 PDT, Boris Zbarsky [:bz]
no flags Details | Diff | Splinter Review
Aviary branch fix... Gotta love forkage (2.13 KB, patch)
2005-05-13 20:18 PDT, Boris Zbarsky [:bz]
no flags Details | Diff | Splinter Review

Description moz_bug_r_a4 2005-05-02 02:29:18 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

Thunderbird doesn't block remote XBL, even though "Block loading of remote
images" setting is true.


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/ixdc-5tn2/test.xml#x);"></p>
</body>


-----
http://members.tripod.com/cv6y-mlr8-9hh/ixdc-5tn2/test.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:label value="This is the remote XBL content."
style="background-color: #f00;"/>
    </content>
  </binding>

</bindings>

Actual Results:  
The remote XBL is loaded.


Expected Results:  
The remote XBL is blocked.
Comment 1 Daniel Veditz [:dveditz] 2005-05-03 13:30:08 PDT
This means XBL loads aren't being checked with any content policies, more of a
core issue (e.g. Adblock wouldn't work against these either). In addition to the
scripting exploit covered in your other bug this lets XBL function as a web-bug
or return-receipt.
Comment 2 Boris Zbarsky [:bz] 2005-05-10 10:35:58 PDT
So I can add a content-policy check in XBL.  Probably should.  But thunderbird
allows RSS to load anything it feels like, so that wouldn't help that much here;
just have to use the RSS feed as an attack vector instead of using an email.
Comment 3 Boris Zbarsky [:bz] 2005-05-10 10:43:25 PDT
Created attachment 183171 [details] [diff] [review]
Add content policy check to XBL

This is compiled; not really tested, because I'm not sure how to test it...
Comment 4 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2005-05-10 10:57:52 PDT
(In reply to comment #2)
> So I can add a content-policy check in XBL.  Probably should.  But thunderbird
> allows RSS to load anything it feels like, so that wouldn't help that much here;
> just have to use the RSS feed as an attack vector instead of using an email.

Not if the attack is determination of valid email addresses by sending email and
waiting for "confirmation".
Comment 5 Boris Zbarsky [:bz] 2005-05-10 11:49:27 PDT
True.  ;)  The posted patch should help with that, I think.
Comment 6 Scott MacGregor 2005-05-10 13:02:22 PDT
I've been able to verify that Boris's patch does work for Thunderbird.
Thunderbird's cotent policy manager is now getting invoked and we are blocking
the remote xbl. I now get a warning at the top of the message saying that
thunderbird has blocked the remote content. 
Comment 7 Boris Zbarsky [:bz] 2005-05-10 13:40:44 PDT
Comment on attachment 183171 [details] [diff] [review]
Add content policy check to XBL

Note that I used the element we're trying to bound to as the context for the
content policy check.  I think that makes more sense than anything else I could
use here...
Comment 8 Johnny Stenback (:jst, jst@mozilla.com) 2005-05-10 13:46:43 PDT
Comment on attachment 183171 [details] [diff] [review]
Add content policy check to XBL

r+sr=jst
Comment 9 Boris Zbarsky [:bz] 2005-05-10 13:48:25 PDT
Comment on attachment 183171 [details] [diff] [review]
Add content policy check to XBL

Requesting 1.0.4 approval, 1.8b2 approval, 1.7.x approval... which I can't,
since this isn't in Core.  :(
Comment 10 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2005-05-10 13:53:39 PDT
Comment on attachment 183171 [details] [diff] [review]
Add content policy check to XBL

Approving for trunk; please try to keep the checkin comment somewhat cryptic
(e.g., don't mention thunderbird).
Comment 11 Daniel Veditz [:dveditz] 2005-05-10 16:32:52 PDT
-->Core
Comment 12 Boris Zbarsky [:bz] 2005-05-10 16:35:02 PDT
Comment on attachment 183171 [details] [diff] [review]
Add content policy check to XBL

Requesting 1.7 approval too.  I've landed this on trunk.
Comment 13 Boris Zbarsky [:bz] 2005-05-11 10:05:40 PDT
That change may break builds with adblock installed... see bug 293778.  Need to
sort out whether it does, and if so why.  Does anyone have an adblock-mangled
debug build?
Comment 14 Boris Zbarsky [:bz] 2005-05-12 22:16:34 PDT
So for branch we'll also want to take the one-liner for bug 293778 (it's a
one-line modification to this patch, basically).

Marking this fixed, since it's fixed on trunk....
Comment 15 Mike Shaver (:shaver -- probably not reading bugmail closely) 2005-05-13 06:05:52 PDT
Comment on attachment 183171 [details] [diff] [review]
Add content policy check to XBL

Approving for stable branches. a=shaver.
Comment 16 Mike Shaver (:shaver -- probably not reading bugmail closely) 2005-05-13 07:38:20 PDT
Comment on attachment 183171 [details] [diff] [review]
Add content policy check to XBL

This should probably get the aContent -> document fix from that other bug too,
before landing on the branches, right?
Comment 17 Boris Zbarsky [:bz] 2005-05-13 08:36:57 PDT
Yes.  That still needs reviews and stuff, though, so I'll hold off on landing on
branches till it gets that.
Comment 18 Boris Zbarsky [:bz] 2005-05-13 20:12:02 PDT
Created attachment 183556 [details] [diff] [review]
1.7 branch fix
Comment 19 Boris Zbarsky [:bz] 2005-05-13 20:18:47 PDT
Created attachment 183558 [details] [diff] [review]
Aviary branch fix... Gotta love forkage
Comment 20 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2005-05-13 21:42:05 PDT
Yeah, I'm glad you caught that the content policy API is completely different
between the two branches (PRBool vs nsresult).  I've hit that a number of times...
Comment 21 Boris Zbarsky [:bz] 2005-05-13 21:45:26 PDT
Fixed on both branches.

Code inspection makes me think that this should work fine for Seamonkey 1.7.9
mailnews, but we should test to make sure once we spin the builds of course.
Comment 22 Boris Zbarsky [:bz] 2005-05-15 21:31:09 PDT
I filed bug 294307 on the issues in the 1.7 mailnews content policy that make
this fix not helpful in 1.7 mailnews.
Comment 23 Jay Patel [:jay] 2005-07-06 16:48:51 PDT
v.fixed on aviary with version 1.0.5 (20050706) using the testcase in comment #0.
Comment 24 Daniel Veditz [:dveditz] 2005-07-12 11:36:02 PDT
Adding distributors
Comment 25 Daniel Veditz [:dveditz] 2005-07-12 18:06:02 PDT
Security advisories published

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