66938 - Eavesdropping on mail messages using innerHTML (wiretap)

Status: VERIFIED FIXED

(Core :: Security, defect, P2)

Description

[Mitchell Stoltz (not reading bugmail)]

A script in a mail message has access to the whole message, including any
content added by forwarders. This means that if A sends a message to B which
contains a malicious script, and B adds some text to the message and forwards it
to C, the script could send B's added text back to A. This is a privacy violation.

The big picture here is that multiple authors can add content to a mail message,
and a script from any one of those authors has access to all the content. A
quick fix would be to block the innerHTML property in mail/news. This is trivial
to do, but it doesn't solve the whole problem - many other DOM properties could
reveal sensitive information about the added content (a list of links or other
tags, for example.)

Comment 1

[Mitchell Stoltz (not reading bugmail)]

Here's a range of possible solutions:

1. A complete fix, of course, would be to disable JS in mail/news messages. I
tend to agree with Richard Smith, the author of this exploit, that JS in mail
adds little value. There have been several recent exploits involving information
leakage from mail messages, and we might want to cut these off at the knees.

2. Restrict DOM access by scripts in messages - the issue here is striking a
balsnce between stopping information leakage and limiting fuynctionality - if we
end up blocking most or all DOM access then we might as well disable JS
completely. If we can block access to all plaintext in the message, I'll be
satisfied - it's probably not too bad if we leak the number and type of HTML
tags, as long as actual text is protected.


3. Just block innerHTML - this is probably not an adequate solution for the long
term but it could be used to defuse a firedrill, as it would block Mr. Smith's case.
  

Comment 2

[ckritzer (gone)]

Good testcase mitch...I'm also working on a testcase to send this info to a
server via email...I'll update the bug later on today...

Comment 3

[ckritzer (gone)]

After chatting with Mitch earlier, I did a little more investigation with the
dom, and it appears that this exploit is not limited to innerHTML.  I have been
able to successfully grab bits and display little bits of the email
HTMLBodyElement (and its children) using dom calls.  I am working on a few
testcases to more easily show this.

Comment 4

[ckritzer (gone)]

Attachment: testcase for innerHTML & DOM

Comment 5

[ckritzer (gone)]

Attachment: ARRGHH! Ignore the last testcase...this is the *real* one...

Comment 6

[ckritzer (gone)]

Added the wrong testcase, and then the correct one.

Steps for the 01/29/01 18:19
(http://bugzilla.mozilla.org/showattachment.cgi?attach_id=23762) testcase:
1) Load testcase.
2) Click innerHTML Exploit link & send mail
3) Repeat step 2) with DOM Exploit link
4) Open your mail and select each email

Results should be:
A) For innerHTML, you will see a popup dialog with a WHOLE lot of text from the
body of the email
B) For DOM, you will see the text of the body appended to the email itself

Comment 7

[ckritzer (gone)]

One SLIGHT clarification of my above comments, concerning the results:

The results I describe are the ones that actually take place, not the ones that
should take place.

Comment 8

[Mitchell Stoltz (not reading bugmail)]

Here's the list of potentially sensitive properties that jst and I came up with.
Chris, can you add to this list?

"characterdata.data", \
"characterdata.substringdata", \
"element.getattribute", \
"element.getattributenode", \
"element.getattributenodens", \
"element.getattributens", \
"htmlanchorelement.href", \
"htmlareaelement.href", \
"htmlbaseelement.href", \
"htmlblockquoteelement.cite", \
"htmlelement.innerhtml", \
"htmlimageelement.src", \
"image.lowsrc", \
"node.attributes", \
"node.nodevalue", \
"nshtmlanchorelement.hash", \
"nshtmlanchorelement.host", \
"nshtmlanchorelement.hostname", \
"nshtmlanchorelement.pathname", \
"nshtmlanchorelement.port", \
"nshtmlanchorelement.protocol", \
"nshtmlanchorelement.search", \
"nshtmlanchorelement.text", \
"nshtmlareaelement.hash", \
"nshtmlareaelement.host", \
"nshtmlareaelement.hostname", \
"nshtmlareaelement.pathname", \
"nshtmlareaelement.port", \
"nshtmlareaelement.protocol", \
"nshtmlareaelement.search", \
"range.tostring", \

Comment 9

[Mitchell Stoltz (not reading bugmail)]

Attachment: Here's that list of properties in the form of a patch to all.js

Comment 10

[lchiang]

unfortunately, ckritzer is out on 1-30.  

Comment 11

[selmer (gone)]

Please check in on the branch ASAP.

Comment 12

[lchiang]

I believe this is checked in already (per emails I've seen) and we have respins
from last night.  1-30-mtest directories.

Comment 13

[lchiang]

leave this assigned until the fix is also in the trunk, I assume.

Comment 14

[lchiang]

Attachment: all.js from 2001-01-30-23-mtest build

Comment 15

[lchiang]

Attachment: all-ns.js

Comment 16

[lchiang]

I created a new profile using the 2001-01-30-23-mtest build which is supposed to
have mstoltz's fix in it.  I don't see any difference in running ckritzer's test
case between the 1-30 build and the previous build.  I attached the js files.  I
don't see all of the attributes/properties that we are blocking.

Pls advise.  I will try another system to be sure.

Comment 17

[lchiang]

I must be losing it.  I'm sorry.  On another machine, the fix is fine (win32
checked so far). Testcases no longer show exploitation.  Will check more.  Pls
disregard what I posted earlier.  Thanks.

Comment 18

[lchiang]

verified testcase is ok on the Mac 2001-01-30-21-mtest build.

Need to verify on Linux still.

Comment 19

[ckritzer (gone)]

Verified Fixed on MacOS 2001-01-30-21 MTEST builds.  Esther said it also works
on the Windows builds.

Comment 20

[ckritzer (gone)]

Attachment: additional tests for the exploit

Comment 21

[ckritzer (gone)]

To run the new additional testcases html page:
1) load page in browser
2) send the page to yourself as an attachment
3 [details] [diff] [review]) open the email

If you can see anything in the table aside from the 'Property' & 'Exploit
Results' headers, the bug still exists

Comment 22

[ckritzer (gone)]

okay, this seems to be working fine on all mtest platforms.

Will check out the tip builds when available.

Comment 23

[esther]

verifed on linux

Comment 24

[Mitchell Stoltz (not reading bugmail)]

The fix that went into NS6.01 was an attempt to prevent access by a script to
the text content of the message while leaving the DOM somewhat functional. The
idea was that if a legitimate user sends a Web page in a message, the majority
of DHTML content on the page will still work (some will inevitably break). Maybe
not the best long-term solution, but I think it was sufficient.

Preventing access to other parts of the message is probably a good thing, but it
doesn't solve the problem. If the forwarder adds to the body of the message and
forwards it inline, rather than forwarding as an attachment, there's no way to
distinguish the original message content from the added content.

The fix is not yet checked into the trunk - I'm waiting to hear if anyone has a
better solution.

Comment 25

[Mitchell Stoltz (not reading bugmail)]

Sorry, the previous post was in reply to this newsgroup post by
[e-mail]:

The fix in the Netscape 6.0x branch seems to be
http://bugzilla.mozilla.org/showattachment.cgi?attach_id=23769 .
Why do we want to do something like that? If we allow JS for
mail at all, why restrict access to e.g. someimage.src?

What we want to prevent is access to other parts of a message.
Thus, the clean way would be to place attachments in an <iframe>
and restrict access to parent.* from within. That would solve
the problems with style too (see
news://news.mozilla.org/3A843605.7971EF63%40clarence.de ).

Comment 26

[lchiang]

mitch - can you assign a target milestone?

Comment 27

[Ben Bucksch (:BenB)]

I suggested the following changes:
- Strip JS and other junk from quotes / inline forwards. This will stop us to
*spread* exploited msgs (inline).
- Remove access between msg parts, e.g. don't let an attachment access the main
body part of the msg. This will save us from *being attacked*, if the sender
forwarded as attachment.

In a "perfect world" (only new Mozillas), the bug will be fixed. But we may
still get msgs from other/older UAs which forwarded the script inline in the msg
main body.

My personal, current :-) opinion to that:
Just disable JS in Mailnews (as we did already in Mozilla and Beonex
Communicator). If somebody explicitly enables JS in Mailnews, he is likely to
want the functionality and will be confused, if part of it doesn't work.
So, the fix suggested here (and shipped with N6.01) is a hotfix which cripples
JavaScript in Mailnews and only causes confusion.

With my suggestion, Mailnews is neither vulnerable (per default) nor does it
spread the exploit other than in attachment-forwards, where it cannot be
prevented (because a major requirement for attachment-forwards is not to alter
the msg).

Comment 28

[Andreas M. "Clarence" Schneider]

This attack is against the sender, not the recipient, although it needs support
by the recipient. Thus, removing scripts from inline quotes will save us from
*being attacked*.

For attachments, only the recipient can prevent the attack. IMHO, it should be
a matter of course that an attachment cannot read other parts of the message.
But we could pop up a warning when the user sends an attachment which contains
scripts. That would be very annoying with attached web pages, so we might limit
the warnings to forwarded messages.

Comment 29

[ckritzer (gone)]

Another suggestion along the lines of Ben & Andreas:

• Default setting for JS in Mail is OFF
• Throw an alert dialog when the user turns JS in Mail ON, warning them they may
be exposing themselves to certain exploits in mail (or some
cleverMarketingWording). 

How much overhead would it be to this?

Comment 30

[Ben Bucksch (:BenB)]

> How much overhead would it be to this?

Coding: Minimal.
Convincing Netscape decision maker (dunno who they are today): Large. See bug 42678.

Comment 31

[Mitchell Stoltz (not reading bugmail)]

I like Ben's suggestions. We should definitely isolate the parts of the message
from each other, that's a start. That and stripping JS from the inline portion
of the message ought to do it, though I'm not sure if we want to do that. JS in
mail should probably be disabled by default, but to play devil's advocate for a
minute:

1) If we're going to allow HTML and webpage attachments in mail messages at all,
they should be fully functional, including the JS. I've heard a lot of people
say that they find JS in mail to be of little value, but obviously someone finds
some value in it, so it should be fully functional.

2) If a feature is available at all, even if it's off by default, we must make
it as secure as possible. Defaulting JS to off hides the problem but doesn't fix it.

I dislike adding new warning dialogs if there's any alternative - people tend to
ignore them or else get turned off to the product if they see too many of those.
With regards to point 2 above, we should fix it instead of warning about it if
that's at all possible.

I think Ben's solution is workable. Ben, can you write a patch for this?

Comment 32

[Andreas M. "Clarence" Schneider]

I dislike adding new warning dialogs too. But we cannot fix Outlook.
Our users can be exploited if the recipients of their mail use Outlook or NS
6.00. Maybe a note at "Understanding Privacy" would be enough.

Comment 33

[Ben Bucksch (:BenB)]

> 1) If we're going to allow HTML and webpage attachments in mail messages at
> all, they should be fully functional, including the JS

Why?
Note: You can easily open the attached webpage in the browser via the
attachments button.

> 2) If a feature is available at all, even if it's off by default, we must make
> it as secure as possible. Defaulting JS to off hides the problem but doesn't
> fix it.

OK, remove it altogether, even the option :-).

Defaulting to JS will fix the problem for the majority (80%?) of users, because
they won't switch the pref in either way.

> Ben, can you write a patch for this?

I think I have the skill to strip JS during quoting / forwarding inline. But I
probably won't have the time. :-(
I don't know how to isolate the msg parts.

> I dislike adding new warning dialogs too. But we cannot fix Outlook.
> Our users can be exploited if the recipients of their mail use Outlook or NS
> 6.00.

I don't understand. What has 'JS in Mailnews' to do with that?

Comment 34

[Ben Bucksch (:BenB)]

> > 2) If a feature is available at all, even if it's off by default, we must
> > make it as secure as possible.

To clarify: I agree. But I don't think, we can make it more secure than I
proposed. And IMO, crippling the functionality (like you did for N6.01) makes no
sense - rather, disable it altogether, then.

[...]
> Defaulting to JS will fix the problem for

s/JS/"JS off"/

Comment 35

[Mitchell Stoltz (not reading bugmail)]

Let's look seriously at Ben's proposal. Here it is again, as I understand it:
1) Isolate message parts so a script in one part has no access to content in
another part. I can do this with some help from the mail team.

2) Strip out, disable, or limit the JS in the inline/quoted portion of the
message. I'm not sure how we determine which part this is, but if we can do both
of these, I think we've solved the problem.

Comment 36

[Ben Bucksch (:BenB)]

As for the impl. of 2): I just remember: Once opon a time, I almost fixed this,
see bug 49021. But the idea has been rejected, and I lost the local change.
Unfortunately, I didn't attach it after closing the bug.
I will roughly describe what I did, see bug 49021.
During quoting (and forwarding, I think), the HTML msg goes through libmime. In
There, you know through same variable, in which mode (display, print, save,
quoting etc.) you are. So, just apply the "filtering" in the quoting/forwarding
modes only. No Gecko magic needed.

Comment 37

[Phil Peterson]

> Isolate message parts so a script in one part has no access to content in
> another part

If you could do that, maybe you could make the headers scroll with the message,
rather than stay fixed as they are now. As I recall, it was only for lack of
insulation between the header area and the body that the headers don't scroll.

Comment 38

[Ben Bucksch (:BenB)]

<offtopic>
> As I recall, it was only for lack of
> insulation between the header area and the body that the headers don't scroll.

Ah! Now I understand, yes, that makes sense.
Because making headers scroll with the msg is, for itself, no problem.

Anyone has the bug number handy?
</offtopic>

Comment 39

[Phil Peterson]

Sorry for the tangent. See bug 9942, bug 32055, bug 47422, bug 71130. In NS4.x,
there was special layout magic to prevent malicious senders from closing the
tags of the generated HTML and using absolute positioning to paint in the header
area. It's the absence of that magic in Gecko which forced us to put the headers
in a separate iframe, and we couldn't figure out how to make the iframes scroll
together based on one scroll bar. 

Comment 40

[Ben Bucksch (:BenB)]

An alternative implementation for 2) might be to let Gecko parse the (to be
quoted) source and them eliminate active content using the DOM. To avoid
double-parsing, we could do that in the Composer. However, we need to make sure
that the content can't do harm between its insertion and removal. Also, there
are several cases how quotes can be inserted (during Reply/Forward,
PasteAsQoutation or 4.x-style Quote command). So, this strategy has its risks, too.

Comment 41

[Mitchell Stoltz (not reading bugmail)]

Didn't get to a permanent fix for 0.9, checked in the temporary fix (the patch
in this bug) so that 0.9 isn't vulnerable to the exploit. Moving milestone to
0.9.l, we really need a better fix for this if possible.

Comment 42

[Mitchell Stoltz (not reading bugmail)]

Attachment: New post-XPCDOM prefs to make mail safer

Comment 43

[Mitchell Stoltz (not reading bugmail)]

jst gave me his r= on this patch, I'm looking for an sr=.

Comment 44

[Scott MacGregor]

I'm good for that much. sr=mscott

Comment 45

[Eric Pollmann]

r=[e-mail]

Comment 46

[Mitchell Stoltz (not reading bugmail)]

Pref change checked in. Leaving this open in the hope that we can still come up
with a better solution.

Comment 47

[Mitchell Stoltz (not reading bugmail)]

Marking Fixed as I think we've done the best we can do. I'll open a new bug to 
track other mail DOM security problems, as a full review is needed for 0.9.2.

Comment 48

[Mitchell Stoltz (not reading bugmail)]

Doh, forgot the most important one. Need to restrict innerHTML. Reopening.

Comment 49

[Mitchell Stoltz (not reading bugmail)]

Target is now 0.9.4, Priority P2.

Comment 50

[Ben Bucksch (:BenB)]

> I'll open a new bug to track other mail DOM security problems,

bug#?

> as a full review is needed for 0.9.2.

Did this happen?

Comment 51

[Mitchell Stoltz (not reading bugmail)]

Nope, that's why the bug is still open. This problem is tricky - possibly
unsolvable. Your help is always appreciated.

Comment 52

[Mitchell Stoltz (not reading bugmail)]

Checked in.

Comment 53

[bsharma]

Verified on 2001-09-20-0.9.4 on WinNT

Ran the InnerHTML exploit, DOM exploit and the other DOM exploit. These messages
are "Get" as is, there is no extra text found.