Last Comment Bug 346984 - xbl and event handlers cause trouble in mailnews and editor
: xbl and event handlers cause trouble in mailnews and editor
Status: RESOLVED FIXED
[sg:high]
: fixed1.8.0.7, fixed1.8.1
Product: Core
Classification: Components
Component: XBL (show other bugs)
: Trunk
: x86 Linux
: P1 major (vote)
: mozilla1.9alpha1
Assigned To: Boris Zbarsky [:bz] (Out June 25-July 6)
: Hixie (not reading bugmail)
Mentors:
: 349342 (view as bug list)
Depends on: 329755 343037 349467
Blocks:
  Show dependency treegraph
 
Reported: 2006-08-02 05:21 PDT by georgi - hopefully not receiving bugspam
Modified: 2007-08-05 23:26 PDT (History)
16 users (show)
dbaron: blocking1.8.1+
dveditz: blocking1.8.0.7+
bzbarsky: in‑testsuite?
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
local folder (1.05 KB, text/plain)
2006-08-02 05:23 PDT, georgi - hopefully not receiving bugspam
no flags Details
xbl file (1.15 KB, text/xml)
2006-08-02 05:23 PDT, georgi - hopefully not receiving bugspam
no flags Details
Super-hacky patch (2.89 KB, patch)
2006-08-20 22:30 PDT, Boris Zbarsky [:bz] (Out June 25-July 6)
mrbkap: review+
jst: superreview+
dveditz: approval1.8.0.7+
Details | Diff | Review
1.8 branch version of patch if we need it (3.39 KB, patch)
2006-08-24 20:20 PDT, Boris Zbarsky [:bz] (Out June 25-July 6)
mtschrep: approval1.8.1+
Details | Diff | Review
1.8.0 branch version of patch (3.40 KB, patch)
2006-08-24 20:21 PDT, Boris Zbarsky [:bz] (Out June 25-July 6)
no flags Details | Diff | Review

Description georgi - hopefully not receiving bugspam 2006-08-02 05:21:11 PDT
xbl and event handlers cause trouble in mailnews and editor

i am not sure how many bugs are in this report, so help split them if needed.

when an image is included via xbl in an email message, its javascript event
handlers (onload,onmouseover) fire in both preview pane and editor (when
forward inline or reply).

so this allows:

1. javascript exploits in email
2. when forwarding or replying, it is possible to steal user's files via
dhtml - img.src='file:///etc/passwd';document.body.appendChild(img);
3. privacy implications because of the image fetching.

affected are thunderbird 1.5.0.5, 2.0a1 and seamonkey trunk at least.

local mailbox with info and xbl file to be attached.
Comment 1 georgi - hopefully not receiving bugspam 2006-08-02 05:23:06 PDT
Created attachment 231742 [details]
local folder
Comment 2 georgi - hopefully not receiving bugspam 2006-08-02 05:23:41 PDT
Created attachment 231743 [details]
xbl file
Comment 3 georgi - hopefully not receiving bugspam 2006-08-02 05:53:01 PDT
for thunderbird Bug 329755
make javascript.enabled=false in thunderbird
stops the exploits (both for 1.5.0.5 and 20a1)
Comment 4 georgi - hopefully not receiving bugspam 2006-08-02 09:24:09 PDT
brendan, i am not flaming on this: can you use your management skills to speed Bug 329755 (it seems to save the birds from this bug) 
Comment 5 Boris Zbarsky [:bz] (Out June 25-July 6) 2006-08-02 15:57:19 PDT
So the basic problem seems to me to be that CompileEventHandler doesn't do any checks at all and CallEventHandler just checks nsJSContext::mScriptsEnabled.  If I understand what that means correctly, event handlers typically run in places where scripts shouldn't be running...  I'm not sure you even need XBL for this testcase, can't you just use an image in the original e-mail (assuming images in mail are enabled)?

Is there a reason we're not checking CanExecuteScripts in CallEventHandler and/or CompileEventHandler?
Comment 6 Brendan Eich [:brendan] 2006-08-02 17:36:08 PDT
(In reply to comment #5)
> Is there a reason we're not checking CanExecuteScripts in CallEventHandler
> and/or CompileEventHandler?

No good reason I know of for CallEventHandler.  We probably shouldn't be testing anything in CompileEventHandler, since compiling is separated in order to share immutable function scripts among windows, so the context on which a handler is compiled will be some early "brutal sharing" context, not a window context in which scripts might be disabled.

/be
Comment 7 Boris Zbarsky [:bz] (Out June 25-July 6) 2006-08-02 17:49:51 PDT
OK.  So perhaps we should just add said check to CallEventHandler....
Comment 8 georgi - hopefully not receiving bugspam 2006-08-03 03:34:54 PDT
(In reply to comment #5)
> If I understand what that means correctly, event handlers typically run in
> places where scripts shouldn't be running...  I'm not sure you even need XBL
> for this testcase, can't you just use an image in the original e-mail (assuming
> images in mail are enabled)?
> 

in mailnews xbl seems needed for this abuse. pure html event handlers (div,input) don't work for me in mailnews - iirc someone killed this bug looong ago.

but if you open an imap uri in seamonkey *browser*, then html event handlers work, though for some reason i can't abuse this for stealing imap email - get sec errors.


Comment 9 Brendan Eich [:brendan] 2006-08-03 11:19:46 PDT
(In reply to comment #7)
> OK.  So perhaps we should just add said check to CallEventHandler....

Agreed -- let's fix this, ASAP.  Dan, I bet this fell on you by default -- can you own it, or find an owner?

/be
Comment 10 georgi - hopefully not receiving bugspam 2006-08-06 02:41:24 PDT
Bug 330443 involves xbl and has privacy implications.

this probably means that at least message content policy is broken.

a potential solution is to *disallow all xbl in mailnews and editor* - i may try to implement this if there is consesus.

Comment 11 Boris Zbarsky [:bz] (Out June 25-July 6) 2006-08-06 15:51:39 PDT
Er... so I checked again, and we do call nsScriptSecurityManager::CheckFunctionAccess which calls nsScriptSecurityManager::CanExecuteScripts.

Then we have, for the mailcompose window:

1)  Script is enabled on the scriptcontext.
2)  Script is enabled on the docshell.
3)  The toplevel docshell is not APP_TYPE_MAIL (!).

So we allow script execution (item #3 means we do not apply the mail-specific JS-enabled pref).  I'm not sure why other event handlers don't execute, but I'm not sure I care.

In any case, the real bug, as far as I'm concerned, is that this is not an APP_TYPE_MAIL window.  Over to the mail folks.
Comment 12 Boris Zbarsky [:bz] (Out June 25-July 6) 2006-08-06 15:52:46 PDT
Er... apparently dveditz owns this component?  ccing people who might know about the compose code.
Comment 13 Daniel Veditz [:dveditz] 2006-08-15 14:13:00 PDT
Let's see if we can get this in. Not likely to be as simple as flipping the default pref (bug 329755) because people who *want* js in mail ("malware hosts") won't have the UI to flip it.
Comment 14 David :Bienvenu 2006-08-15 20:47:56 PDT
I believe the app type is EDITOR in this situation - we set it explicitly in several places, e.g.,

http://lxr.mozilla.org/mailnews/source/mailnews/compose/src/nsMsgCompose.cpp#501

One potential reason for this is to allow images in compose window, even though they're blocked in mail messages. See https://bugzilla.mozilla.org/show_bug.cgi?id=206793 (though I'm not sure the patch in there is checked in)

We could try setting the app type to MAIL and see what happens. We could also implement disabling js in editor apps like we do for mail apps, or add a separate pref for that and set it ito false n mailnews.js, and leave it on for real editor apps.
Comment 15 Boris Zbarsky [:bz] (Out June 25-July 6) 2006-08-15 21:02:49 PDT
I think disabling JS in general for editor apps is a good idea, actually.  You wouldn't want scripts running while the editor is editing stuff!
Comment 16 Scott MacGregor 2006-08-15 21:28:56 PDT
David, I don't think change the app type is going to get you very far. I think that's just going to cause us to go through nsMsgContentPolicy which is not setup to properly cope with compose windows. I've been trying go fix this same problem in Bug 330443 but it is pretty hard. I doubt it's something we'd take in a point release unless a much simpler solution appears than what I've been working on.
Comment 17 Daniel Glazman (:glazou) 2006-08-15 23:52:59 PDT
(In reply to comment #15)

> I think disabling JS in general for editor apps is a good idea, actually.  You
> wouldn't want scripts running while the editor is editing stuff!

Well.... Who knows ? Why couldn't a binding retrieve data from a database and
dynamically insert it into an editor ?

If I agree 100% that xbl should be disabled in emails messages because this
is a security threat, I clearly see how xbl can be used in a markup editor like
Nvu ; adding bindings to the edited html elements is an extension for the editor
itself, while xpis can only extend the chrome (right, right, you can always
upgrade the core but that's not a game I am willing to play).

To give you a concrete example, the first implementation of image resizing
in the editor I did was using XBL. It was extremely fast and reliable.
It was beautiful, really.
Comment 18 Boris Zbarsky [:bz] (Out June 25-July 6) 2006-08-15 23:54:32 PDT
This comes back to chrome vs non-chrome XBL.  We should allow chrome XBL in editor.  We should not allow arbitrary XBL.
Comment 19 David :Bienvenu 2006-08-16 10:41:18 PDT
it's not clear to me that "javascript.enabled" overrides "javascript.allow.mailnews" - this code seems to check if they're different, and if it's a mail window, use the mail pref:

http://lxr.mozilla.org/seamonkey/source/caps/src/nsScriptSecurityManager.cpp#1629

If I add a "javascript.allow.editor" and make it mirror the behavior of "javascript.allow.mailnews" for windows of type APP_TYPE_EDITOR, willthat work for folks? As I said before, TB can set that pref to false, and nVu can do what it wants (not sure about SM - it might want to allow it for composer but not for mail compose...but at least it's under user control, and we can default to the safe choice)
Comment 20 Boris Zbarsky [:bz] (Out June 25-July 6) 2006-08-16 11:12:39 PDT
> If I add a "javascript.allow.editor" and make it mirror

This is what I think, yes.
Comment 21 David :Bienvenu 2006-08-17 10:32:38 PDT
I tried this on the mac - as near as I can tell, the docshell we're looking at is for the hidden window, which is of type unknown (this is in the message preview pane, handling a mouse over event, so the problem seems much worse). Is that the docshell/window we'd expect?

I'll try this on windows, and look at the compose window as well.
Comment 22 Boris Zbarsky [:bz] (Out June 25-July 6) 2006-08-17 13:28:55 PDT
In the message preview pane the handler's not called; when I last checked the docshell there was the message preview docshell, which has the right app type...

I'm not sure what's going on with Mac, though, given the way menus work there.  Does the JS execute on Mac in the message preview?
Comment 23 David :Bienvenu 2006-08-17 13:35:43 PDT
yes, the js executes on both the mac and windows. I was able to verify that it was the hidden window on the mac; it was a bit harder to match up the doc shell pointer values on windows to see if it was the hidden window, but when I set the hidden window's app type to MAIL, the js stopped executing, both in the preview pane, and the compose window, on windows.

Just so I understand, in this case, it shouldn't be the hidden window's docshell that we get from the nsIScriptGlobalObject referred to by js context, right?
Comment 24 Boris Zbarsky [:bz] (Out June 25-July 6) 2006-08-17 19:38:42 PDT
OK, I can reproduce the preview pane issue -- I had to tell seamonkey to load the remote content.

And yes, the script is running on the hidden window, which is mucho wrong.  Here's what happens:

1)  We parse the XBL.
2)  We clone the <img> node when installing XBL anonymous content.
3)  Cloning sets the on* attributes on the new node.
4)  Setting on* attributes calls into
    nsEventListenerManager::AddScriptEventListener
5)  This looks for an nsIScriptGlobalObject to use.
6)  The document's script global is still null, since the owner doc at this
    point is the XBL document.  So we fall back to the "get JS context from the
    stack, or safe JS context" thing.  Luckily, there's no chrome on the stack,
    so we get the hidden window context (safe context).
7)  We store this context in our listener.
8)  Later on, we check whether we can execute the listener and pass the context
    to the security check.  The security manager uses this to get to the
    docshell.
9)  It finds the hidden window, which is not of TYPE_MAIL and has scripts
    enabled.

I tried applying the patch from bug 347523 and commenting out the failing security check in ImportNode, and that fixed the bug.  The event handlers now had the right JSContext, and the mail preview is TYPE_MAIL, while the compose window's docshell has script disabled.

So I think we should do the following, short-term:

A) Introduce an nsIDocument (_MOZILLA_1_8_BRANCH on branches) API to do
   ImportNode but without the security check.  Alternately, push a chrome
   subject principal in XBL before importing, but I'm not sure we can do that,
   and doing it bothers me in general.
B) Use this for XBL.
C) Add some "XXX sXBL/XBL2" comments in AddScriptEventListener.
D) If possible, remove the code in
   nsEventListenerManager::AddScriptEventListener that fishes for a script
   context.  Having random code looking at the JSContext stack and giving
   things principals based on that stack just because it's in a document that
   has no scripting environment bothers me...  We might need to deal on trunk
   with AdoptNode on a node coming from such a document and event handlers
   being broken on it, but that should be fixable.
Comment 25 Boris Zbarsky [:bz] (Out June 25-July 6) 2006-08-17 19:54:40 PDT
Looking at CVS blame, the log entry for the checkin that added the "look at the js stack" thing is:

3.121 <saari@netscape.com> 2000-08-08 14:30
massive landing of joki changes.
Relevant nsbeta3+ bugs 43309, 44503, 2634, 2504,5981, 24698, 25758, 33577,
36062, 36217, 41191, 41491, 42356, 42829, 43016
r=saari (joki code). also been tested by heikki and bryner

I'm guessing the js stack thing was added to fix bug 33577, which is a non-issue now because createContextualFragment creates nodes with the right ownerDocument.  So I stand by removing it, at least on trunk.
Comment 26 David :Bienvenu 2006-08-17 21:42:26 PDT
Thx for figuring this out! Who's the right person to do the short term work for 1.5.0.x?

Out of curiousity, why does the compose window's docshell have script disabled? It's not of type APP_MAIL...
Comment 27 Boris Zbarsky [:bz] (Out June 25-July 6) 2006-08-17 22:34:41 PDT
Another option is to change the cx and such on the event handlers of a node when adopting it (whether through adoptNode or the auto-adopt we do right now).  We probably want that for adoptNode, in any case, even if we fix XBL using an importNode hack.

I could do the work once we decide which approach makes us happier, I guess...  Peter, Johnny, Jonas, what do you think?

> Out of curiousity, why does the compose window's docshell have script
> disabled?

Probably because of:

  /editor/composer/src/nsEditingSession.cpp, line 164 -- 
    rv = docShell->SetAllowJavascript(PR_FALSE);

Comment 28 georgi - hopefully not receiving bugspam 2006-08-19 04:44:55 PDT
(In reply to comment #13)
> Let's see if we can get this in. Not likely to be as simple as flipping the
> default pref (bug 329755) because people who *want* js in mail ("malware
> hosts") won't have the UI to flip it.
> 

i suspect that people that want to shoot themselves by enabling js in mail will cry when they get 0wn3d, so you at least can point them they should *not* enable dangerous preferences especially when there is deliberately no UI for shooting.

if they like shooting themselves so much, they can ask the sender for attaching the HTML/XML and then it will run with 'file:///' URI (as of 2006/08/19).
Comment 29 georgi - hopefully not receiving bugspam 2006-08-19 05:00:27 PDT
(In reply to comment #24)
> OK, I can reproduce the preview pane issue -- I had to tell seamonkey to load
> the remote content.
> 

i am not sure this is entirely correct.

by default thunderbird loads remote content if the sender is the personal address book - tested this with new local linux account and installed the bird.
so if the sender is in the address book, xbl fires js.

and email spoofing is not rocket science.


Comment 30 georgi - hopefully not receiving bugspam 2006-08-19 05:17:24 PDT
bz: in the previous comment i meant: if you add the sender of 'local folder' to the personal address book of thunderbird 1.5, does the js fire automatically in preview pane?
Comment 31 David :Bienvenu 2006-08-19 07:13:20 PDT
by default, you're right - if the sender is in your pab, we won't block remote content like images.
Comment 32 Boris Zbarsky [:bz] (Out June 25-July 6) 2006-08-19 08:39:55 PDT
G30rgi, in case you missed it I wasn't even talking about thunderbird.  The reason I mentioned that at all is so that I would have recorded steps to reproduce when I tried to do so again.
Comment 33 georgi - hopefully not receiving bugspam 2006-08-19 12:08:14 PDT
(In reply to comment #32)
> G30rgi, in case you missed it I wasn't even talking about thunderbird.  The
> reason I mentioned that at all is so that I would have recorded steps to
> reproduce when I tried to do so again.
> 

bz, i didn't mean to flame with you :)

should i file another bug for this default behaviour in thunderbird or change the Component?

with a trunk, a fox, a bird and a lot of branches, it is not very clear against what to file a bug.

Comment 34 georgi - hopefully not receiving bugspam 2006-08-19 12:24:58 PDT
*** Bug 349342 has been marked as a duplicate of this bug. ***
Comment 35 georgi - hopefully not receiving bugspam 2006-08-19 12:27:46 PDT
the dupe of this for thunderbird is Bug 349342
Comment 36 Boris Zbarsky [:bz] (Out June 25-July 6) 2006-08-20 21:47:28 PDT
OK, importNode is not even implemented on the branch... So we might as well do the cx changing, I guess.  :(  It's probably safer, and we want it on trunk too...
Comment 37 Boris Zbarsky [:bz] (Out June 25-July 6) 2006-08-20 22:09:37 PDT
So what we need is some new API on nsIJSEventListener and on nsIEventListenerManager to change contexts.... and a way to enumerate all the listeners for a node.  Most are not too bad, but eEventArrayType_Hash sucks... :(

I'll see if I can think of something hacky and simple, I guess, and hope that on trunk life becomes better.
Comment 38 Boris Zbarsky [:bz] (Out June 25-July 6) 2006-08-20 22:28:24 PDT
Filed bug 349465 on changing the cx in existing event listeners and bug 349467 on the nsEventListenerManager changes.
Comment 39 Boris Zbarsky [:bz] (Out June 25-July 6) 2006-08-20 22:30:37 PDT
Created attachment 234737 [details] [diff] [review]
Super-hacky patch

This does fix the bug....  I hope it doesn't break anything.  :(
Comment 40 Boris Zbarsky [:bz] (Out June 25-July 6) 2006-08-20 22:31:36 PDT
Note that I'll need to merge to branch, due to DOM_AGNOSTIC.  But I used nsIContent instead of nsINode to make that a little easier...
Comment 41 georgi - hopefully not receiving bugspam 2006-08-20 23:10:08 PDT
i am not familiar with the code, just judging from comments, so probably this is nonsense:

as far i can understand, part of the problem is 'hidden window of type unknown'. unless such windows are very common, can the following work:

1. if the caller is chrome, allow js access
...
2. if the window is 'hidden window of type unknown' disallow js access.

in this scenario even if something breaks, it may be a sign of a problem.
Comment 42 Boris Zbarsky [:bz] (Out June 25-July 6) 2006-08-21 08:07:47 PDT
> 2. if the window is 'hidden window of type unknown' disallow js access.

Sadly, given the current state of XBL, that would break the entire browser UI... And yes, it's a sign of a problem; see bug 349465.
Comment 43 Blake Kaplan (:mrbkap) (please use needinfo!) 2006-08-22 17:21:36 PDT
Comment on attachment 234737 [details] [diff] [review]
Super-hacky patch

Seems icky, but reasonable. The control flow in this function is a bit weird, but I guess I can just blame C++ not having a finally construct for that.
Comment 44 Jonas Sicking (:sicking) PTO Until July 5th 2006-08-23 11:44:40 PDT
Does my patch in bug 343037 by any chance help here at all? It should make it impossible to do things like put onclick attributes on elements inside xbl bindings in mail
Comment 45 Boris Zbarsky [:bz] (Out June 25-July 6) 2006-08-23 12:09:22 PDT
Hmm... it might help, maybe... you want to test?  If not, I'll try to apply it and build it tonight.
Comment 46 Johnny Stenback (:jst, jst@mozilla.com) 2006-08-23 15:13:24 PDT
Comment on attachment 234737 [details] [diff] [review]
Super-hacky patch

sr=jst whether we need this or not.
Comment 47 Boris Zbarsky [:bz] (Out June 25-July 6) 2006-08-23 20:56:49 PDT
> Does my patch in bug 343037 by any chance help here at all?

Yes, it does.  Should we do that instead of this hack, then?  I think I would prefer it.
Comment 48 Daniel Veditz [:dveditz] 2006-08-24 11:39:51 PDT
Bug 343037 appears inappropriate for 1.8.0.7 given that it likely breaks themes. Would the "super-hacky patch" be OK enough for 1.8.0.x?
Comment 49 Boris Zbarsky [:bz] (Out June 25-July 6) 2006-08-24 14:46:34 PDT
Comment on attachment 234737 [details] [diff] [review]
Super-hacky patch

Yeah, let's do this for the stable branch.
Comment 50 Boris Zbarsky [:bz] (Out June 25-July 6) 2006-08-24 14:50:49 PDT
I checked this in on trunk to get some bake-time.  We should back it out once bug 343037 lands on trunk.
Comment 51 Daniel Veditz [:dveditz] 2006-08-24 15:28:22 PDT
Comment on attachment 234737 [details] [diff] [review]
Super-hacky patch

a=dveditz for 1.8.0 branch
Comment 52 Boris Zbarsky [:bz] (Out June 25-July 6) 2006-08-24 20:20:53 PDT
Created attachment 235353 [details] [diff] [review]
1.8 branch version of patch if we need it
Comment 53 Boris Zbarsky [:bz] (Out June 25-July 6) 2006-08-24 20:21:46 PDT
Created attachment 235354 [details] [diff] [review]
1.8.0 branch version of patch
Comment 54 Boris Zbarsky [:bz] (Out June 25-July 6) 2006-08-24 20:24:02 PDT
Checked in on 1.8.0 branch for 1.8.0.7.
Comment 55 Jay Patel [:jay] 2006-09-07 12:33:01 PDT
G30rgi:  Can you possibly verify this fix with the latest Thunderbird 1.5.0.7 candidate builds?  Thanks!
Comment 56 georgi - hopefully not receiving bugspam 2006-09-08 00:22:58 PDT
(In reply to comment #55)
> G30rgi:  Can you possibly verify this fix with the latest Thunderbird 1.5.0.7
> candidate builds?  Thanks!
> 

according to tests it is fixed in thunderbird version 1.5.0.7 (20060907)
Comment 57 Daniel Veditz [:dveditz] 2006-09-11 11:40:50 PDT
I doubt bug 343037 will be landing in time for Tbird2.0 final, can we take this approach on the 1.8 branch also?
Comment 58 Boris Zbarsky [:bz] (Out June 25-July 6) 2006-09-11 14:27:07 PDT
Comment on attachment 235353 [details] [diff] [review]
1.8 branch version of patch if we need it

Yeah, I guess we should.
Comment 59 Mike Schroepfer 2006-09-11 15:39:00 PDT
Comment on attachment 235353 [details] [diff] [review]
1.8 branch version of patch if we need it

a=schrep for drivers.
Comment 60 Daniel Veditz [:dveditz] 2006-09-13 17:35:32 PDT
From the summary: "...trouble in mailnews and editor"

In Thunderbird 1.5.0.7 this patch fixes the "editor" part--reply or forward inline--but not the "mailnews" part. If you load the XBL the scripts still run when you're viewing the message (in both three-pane and standalone message windows).
Comment 61 Boris Zbarsky [:bz] (Out June 25-July 6) 2006-09-13 19:16:53 PDT
Argh.  I'd tested in Seamonkey mailnews, and this patch does fix the bug there.  I thought the two were similar enough...

Pulling and building Thunderbird now.
Comment 62 Boris Zbarsky [:bz] (Out June 25-July 6) 2006-09-13 19:39:50 PDT
So... I just pulled thunderbird from the 1.8.0 branch and built it... and I don't see the bug in that build.

I also can't reproduce the problem in the build at ftp://ftp.mozilla.org/pub/mozilla.org/thunderbird/nightly/2006-09-13-03-mozilla1.8.0/thunderbird-1.5.0.7.en-US.linux-i686.tar.gz

dveditz, what exact build were you testing?
Comment 63 Daniel Veditz [:dveditz] 2006-09-13 19:45:24 PDT
I tested both the 1.5.0.5 windows release build as well as the Thunderbird 1.5.0.7 rc6 candidate build.  The reply/forward problem was fixed in .0.7 and the message window problem was seen in both.

I don't think Georgi wrote this bug based on windows testing (references /etc/passwd) and he was seeing it too.
Comment 64 Daniel Veditz [:dveditz] 2006-09-13 19:56:41 PDT
So I am an idiot. My "clean" profile was not quite as virgin as I thought, at some point I allowed javascript in mail -- so of course I got javascript in mail.

This *is* fixed.
Comment 65 Boris Zbarsky [:bz] (Out June 25-July 6) 2006-09-13 19:59:36 PDT
Checked in on 1.8 branch.  I missed that approval somehow...
Comment 66 Daniel Veditz [:dveditz] 2006-09-13 20:48:21 PDT
Not sure how to rate this one. JavaScript in mail is an option people can actually choose to inflict on themselves, so how bad can it be? On the other hand  it makes people vulnerable to all the browser flaws they thought they were protected from by not having script in mail.

The most direct attack here is in the ability to "mail-tap" a reply or forward. Assuming a highly sensitive comment added to a forward the attack is a "high impact" information stealing one. But if there were a critical browser bug then this turns it into a critical mail bug as well.

stepping-stone bugs are always hard to rate :-(
Comment 67 georgi - hopefully not receiving bugspam 2006-09-14 00:02:40 PDT
(In reply to comment #66)

> 
> The most direct attack here is in the ability to "mail-tap" a reply or forward.
> Assuming a highly sensitive comment added to a forward the attack is a "high
> impact" information stealing one. But if there were a critical browser bug then
> this turns it into a critical mail bug as well.

don't consider this critical.

but the above quote is not quite correct - hitting reply or forward inline allows stealing local files (/etc/passwd in the testcase) - reply to the testcase and examine the source of the message - content /etc/passwd is there.



Comment 68 georgi - hopefully not receiving bugspam 2006-09-14 00:19:02 PDT
(In reply to comment #66)
> But if there were a critical browser bug then
> this turns it into a critical mail bug as well.

a little nit - critical *javascript based* browser bug - this doesn't affect non javascript bugs, though may make reliability of exploit higher with the help of javascript.
Comment 69 georgi - hopefully not receiving bugspam 2006-09-15 04:21:34 PDT
couldn't make this work via mailto: urls - is this by design or am i missing something.

looks like mailto: does not invoke styles in editor at all.
Comment 70 Boris Zbarsky [:bz] (Out June 25-July 6) 2007-03-28 13:38:16 PDT
At what point can we open up this bug?

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