Last Comment Bug 319858 - javascript execution when forwarding or replying (CVE-2006-0884)
: javascript execution when forwarding or replying (CVE-2006-0884)
Status: RESOLVED FIXED
[sg:critical] mail bug [qa:verified-t...
: fixed1.8.1, verified1.7.13, verified1.8.0.2
Product: MailNews Core
Classification: Components
Component: Composition (show other bugs)
: 1.7 Branch
: x86 Linux
: -- normal (vote)
: ---
Assigned To: Scott MacGregor
:
Mentors:
Depends on: 327078
Blocks:
  Show dependency treegraph
 
Reported: 2005-12-11 04:49 PST by georgi - hopefully not receiving bugspam
Modified: 2008-07-31 04:30 PDT (History)
6 users (show)
dveditz: blocking1.7.13+
dveditz: blocking‑aviary1.0.8+
dveditz: blocking‑thunderbird2+
dveditz: blocking1.8.0.2+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
local folder (1.77 KB, text/plain)
2005-12-11 04:51 PST, georgi - hopefully not receiving bugspam
no flags Details
local folder accessing /etc/passwd in iframe when forward as inline or edit as new (759 bytes, text/plain)
2006-02-24 00:21 PST, georgi - hopefully not receiving bugspam
no flags Details
additional patch for 1.0.8 (1003 bytes, patch)
2006-02-24 15:27 PST, Scott MacGregor
dveditz: approval‑aviary1.0.8+
mscott: approval1.7.13+
Details | Diff | Splinter Review

Description georgi - hopefully not receiving bugspam 2005-12-11 04:49:34 PST
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8) Gecko/20051111 Firefox/1.5
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8) Gecko/20051111 Firefox/1.5

it is possible to inject javascript in thunderbird 1.0.7 and 1.5rc.
the js is executed if the luser selects "forward as inline" (1.5rc) or
"forward as inline" or "reply" (1.0.7).

this has at least privacy implications for tracking the body of forwarded
messages (e.g. indian developers sniffing neocolonializm).

eventually may lead to side effects (js console shows strange errors).

attached local folder.




Reproducible: Always

Steps to Reproduce:
instructions in local folder
Comment 1 georgi - hopefully not receiving bugspam 2005-12-11 04:51:12 PST
Created attachment 205542 [details]
local folder

local folder
Comment 2 Daniel Veditz [:dveditz] 2005-12-14 18:05:25 PST
Need to investigate the context and make sure chrome execution is not possible.
Comment 3 David :Bienvenu 2005-12-14 21:17:54 PST
similar things will happen if you try to edit this message as new, since it goes through the same code.

When you do a forward inline, we create a mime stream converter, from message/rfc822, and call AsyncConvertData with an input channel that retrieves the message source.

"?from=message/rfc822&to=application/vnd.mozilla.xul+xml"

We then insert the html into the editor, and I think it's the editor that parses the html and causes the script to get run. I don't know if that makes it a bug in editor, or if we should be stripping out data urls before handing them to editor...but remember, this code is used for edit as draft, so we try to keep the html around...
Comment 4 georgi - hopefully not receiving bugspam 2005-12-14 23:57:16 PST
i tried to access editor's methods/properties, but couldn't so far.

if editor gets exposed, things may not be very nice.

on 1.0.7 javascript retrieved from http works iirc, so filtering the "data" protocol for http may not be solution for 1.0.7.
Comment 5 Daniel Veditz [:dveditz] 2006-01-19 10:12:42 PST
Thunderbird just released its 1.5 and given the timing is going to sync up at 1.5.0.2
Comment 6 Scott MacGregor 2006-01-23 15:25:21 PST
Dan, if the javascript isn't running with chrome priveleges, how severe does this become? 

There are easier ways with remote images when you reply or forward a message with remote parts to track privacy than this, so it seems less interesting from a privacy point of view. And if the script can't actually get to chrome...

On a side note, I've been trying to see if our policy capabilities apply to editor so you could do things like: 

capability.policy.editor...., "NoAccess"

I don't think that's going to work. I'll keep looking at some other way to disable javascript from editor shells. 
Comment 7 Scott MacGregor 2006-01-23 16:01:30 PST
Hmm, according to what I'm seeing in the debugger, the Alert is being opened by some JS that's running with the system principal. That's bad times.

I think editor just tells the parser to create a document fragement for the passed in text and that fragment creation must get the system principal and run the JS. 


Comment 8 Scott MacGregor 2006-01-23 18:15:15 PST
It looks like editor intentionally disables javascript in a given editing session:

http://lxr.mozilla.org/seamonkey/source/editor/composer/src/nsEditingSession.cpp#163

However, nsScriptSecurityManager::CanExecuteScripts, sees that we are using the system principal and allows the script to run, even though the docshell is properly flagged as not supporting javascript.

Comment 9 Tim Riley [:timr] 2006-02-10 11:50:49 PST
Too late for 1.0.8
Comment 10 Daniel Veditz [:dveditz] 2006-02-22 09:15:59 PST
This issue has been independently reported at http://sysdream.com/article.php?story_id=230&section_id=78

-> critical based on comment 7
Comment 11 Boris Zbarsky [:bz] 2006-02-22 11:31:06 PST
Scott, what caller is passing a system principal to CanExecuteScripts, exactly?
Comment 12 Scott MacGregor 2006-02-22 13:30:48 PST
nsScriptLoader::EvaluateScript is getting the system principal from mDocument->GetNodePrincipal.

The URI for mDocument is the data url we are running as part of the <iframe src="data://..."> from the message body being inserted into editor.

The owner document of the iframe is about:blank because that's the initial src we load into an empty editor instance:

http://lxr.mozilla.org/mozilla/source/mail/components/compose/content/messengercompose.xul#835

The resulting DOM looks like:

vbox
 editor
   html document whose base URI is about:blank
     body
       iframe whose src is the data url

I'll keep digging to see why we end up getting the system principal in this scenario.  
Comment 13 Boris Zbarsky [:bz] 2006-02-22 13:58:33 PST
Scott, does the patch in bug 327078 help?  I bet it should given comment 12 if the parsing is all happening while called from chrome.
Comment 14 Scott MacGregor 2006-02-22 15:32:11 PST
Wow, I love it when someone has already fixed the problem for me. That patch does indeed address this issue. 

The principal associated with the iframe is no longer the system principal. As a result ::CanExecuteScript now finds that the docshell has JS disabled, and the script fails to execute.

Thanks for the help Boris.
Comment 15 georgi - hopefully not receiving bugspam 2006-02-23 02:43:43 PST
when selecting "reply" is the principal still system?

can't access Components.classes in this case and get js error.
Comment 16 georgi - hopefully not receiving bugspam 2006-02-23 03:14:00 PST
this is related to Bug 319859
Comment 17 georgi - hopefully not receiving bugspam 2006-02-23 07:02:46 PST
<iframe src='file:///etc/passwd'> works when "forward as inline" on trunk and 1.5
Comment 18 Scott MacGregor 2006-02-23 17:14:52 PST
When I replied to the test message, the principal was still not the system principal, and the JS failed to execute. 
Comment 19 Scott MacGregor 2006-02-23 20:17:03 PST
Unfortunately the patch in Bug 327078 doesn't fix this issue on the 1.0.8 branch. We'll need more changes than just that if we take this there.

For 1.0.8, editor disables javascript on the script context itself, but the security manager gets the global object for the same script context, hunting for docshells with JS disabled. So scripts still run.

We'll probably need to add the patches that made editor start modifying the docshell JS enabled property instead of the script context if we fix this for 1.0.8.

I'll keep digging.
Comment 20 Boris Zbarsky [:bz] 2006-02-23 21:06:16 PST
Er... somehow my last comment went missing.  :(  Scott, what's the stack for us calling into CanExecuteScripts in this case?  I want to make sure the patch in bug 327078 ends up correctly handling some non-chrome cases too...

As for 1.7 branch, we could also just take the part of the fix for bug 325991 that changed nsScriptSecurityManager::CanExecuteScripts, no?  Revision 1.284 of nsScriptSecurityManager.cpp.  That should help, no?
Comment 21 Scott MacGregor 2006-02-23 23:47:21 PST
(In reply to comment #20)
> Er... somehow my last comment went missing.  :(  Scott, what's the stack for us
> calling into CanExecuteScripts in this case?  I want to make sure the patch in
> bug 327078 ends up correctly handling some non-chrome cases too...
> 
> As for 1.7 branch, we could also just take the part of the fix for bug 325991
> that changed nsScriptSecurityManager::CanExecuteScripts, no?  Revision 1.284 of
> nsScriptSecurityManager.cpp.  That should help, no?

That change to nsScriptSecurityManager looks like it should fix what I am seeing on the 1.0.x branch. I'll confirm that in the morning to be sure.

I'll post a stack trace leading into CanExecuteScripts tomorrow morning as well. 

Comment 22 georgi - hopefully not receiving bugspam 2006-02-24 00:21:09 PST
Created attachment 213016 [details]
local folder accessing /etc/passwd in iframe when forward as inline or edit as new
Comment 23 georgi - hopefully not receiving bugspam 2006-02-24 00:22:32 PST
does the patch fix accessing /etc/passwd in editor?

if not, i shall file a new bug.
Comment 24 Daniel Veditz [:dveditz] 2006-02-24 01:52:18 PST
(In reply to comment #23)
> does the patch fix accessing /etc/passwd in editor?
> if not, i shall file a new bug.

That bit sounds like bug 303752 (which I've just cc'd you on), unless for some reason iframe src= is checked differently than img src=. If it's not the same issue (which should be fixed in recent ff1.0.8 builds) then please do file a new bug.
Comment 25 georgi - hopefully not receiving bugspam 2006-02-24 02:19:27 PST
> 
> That bit sounds like bug 303752 (which I've just cc'd you on), unless for some
> reason iframe src= is checked differently than img src=. If it's not the same
> issue (which should be fixed in recent ff1.0.8 builds) then please do file a
> new bug.
> 

i am not sure it is the same - iframe doesn't seems to attach the file, but if js is managed to be executed, the file may be accessed. accessing local files in general is not very good idea.

ff1.0.8??? - this is mail problem - the iframe seems to work on thunderbird trunk.

so should i file a bug - in this bug there is testcase?


Comment 26 georgi - hopefully not receiving bugspam 2006-02-24 03:01:02 PST
regarding js and iframe:

for tb 1.5 and trunk at least a kludge may be to make "forward as inline" act as "reply" if possible. "reply" seems to work correctly.
Comment 27 Daniel Veditz [:dveditz] 2006-02-24 03:23:45 PST
(In reply to comment #19)
> Unfortunately the patch in Bug 327078 doesn't fix this issue on the 1.0.8
> branch. We'll need more changes than just that if we take this there.

Seems to fix it in my tbird 1.0.x tree (which is a week or so old). Let me update to the latest and retest.

(In reply to comment #25)
> ff1.0.8??? - this is mail problem - the iframe seems to work on thunderbird
> trunk.

Mistyped, meant thunderbird 1.0.8 of course.
http://ftp.mozilla.org/pub/mozilla.org/thunderbird/nightly/latest-aviary1.0.1/

> so should i file a bug - in this bug there is testcase?

It's a separate problem that will require a separate fix, so yes (you don't have to duplicate the testcase, just link to the one here). When problems get combined into the same bug bits get missed or don't get properly regression tested in later releases. Especially when the different bits of the bug have different branch impacts as this one seems to.

I don't see any problem in 1.0.7 -- there's a js console error saying "about:blank may not load or link to file:///" (img src is still sent on fwd, but not reply -- that's 303752). In 1.5 the fwd inline *displays* the frame content which is disturbing (and unlike 1.0.x) but the content is not actually sent when I try it--just as a locally sourced img is displayed but not sent (again, bug 303752).
Comment 28 georgi - hopefully not receiving bugspam 2006-02-24 03:56:53 PST
(In reply to comment #27)

ok, will file a new bug.

indeed, 1.0.7 and 1.0.8 seem safe from iframe.
Comment 29 georgi - hopefully not receiving bugspam 2006-02-24 04:09:38 PST
filed Bug 328445
Comment 30 Daniel Veditz [:dveditz] 2006-02-24 04:14:21 PST
(In reply to comment #27)
> Seems to fix it in my tbird 1.0.x tree (which is a week or so old). Let me
> update to the latest and retest.

I had a modified testcase: The patch from bug 327078 made my alert(Components.classes) testcase stop working, but otherwise didn't stop scripts from running. The nsScriptSecurityManager portion of bug 325991 didn't help (though seems like a good idea anyway). The fix for bug 280120 also modified nsScriptSecurityManager::CanExecuteScripts between 1.7 and 1.8, but that didn't help either.
Comment 31 georgi - hopefully not receiving bugspam 2006-02-24 04:52:34 PST
"reply" and "forward as inline" seem to have similar functionality and probably the implementation is not *very* different.

"reply" is safe on 1.5.

can the difference be located and as a kludge forward be fixed the way reply is?

may be wrong.
Comment 32 Scott MacGregor 2006-02-24 12:31:20 PST
Here's the promised stack trace for Boris showing the call into CanExecuteScripts for the iframe whose src is the JS URL in question.

>	nsScriptSecurityManager::CanExecuteScripts() Line 1534	C++
 	nsDocument::IsScriptEnabled() Line 4670	C++
 	nsScriptLoader::DoProcessScriptElement() Line 370	C++
 	nsScriptLoader::ProcessScriptElement() Line 342	C++
 	nsHTMLScriptElement::MaybeProcessScript() Line 700	C++
 	nsHTMLScriptElement::DoneAddingChildren() Line 562	C++
 	HTMLContentSink::ProcessSCRIPTEndTag() Line 3924	C++
 	SinkContext::CloseContainer() Line 1406	C++
 	HTMLContentSink::CloseContainer() Line 2972	C++
 	CNavDTD::CloseContainer() Line 2677	C++
 	CNavDTD::HandleEndToken() Line 1586	C++
 	CNavDTD::HandleToken() Line 698	C++
 	CNavDTD::BuildModel() Line 331	C++
 	nsParser::BuildModel() Line 1752	C++
 	nsParser::ResumeParse() Line 1629	C++
 	nsParser::OnDataAvailable() Line 2266	C++
 	nsDocumentOpenInfo::OnDataAvailable() Line 361	C++
 	nsBaseChannel::OnDataAvailable() Line 597	C++
 	nsInputStreamPump::OnStateTransfer() Line 476	C++
 	nsInputStreamPump::OnInputStreamReady() Line 366	C++
 	nsInputStreamReadyEvent::EventHandler() Line 121	C++
 	PL_HandleEvent() Line 688	C
 	PL_ProcessPendingEvents() Line 623	C
 	_md_EventReceiverProc() Line 1408	C

Comment 33 Boris Zbarsky [:bz] 2006-02-24 13:23:16 PST
Er... in that case, why does the patch for bug 327078 help?  I don't see an JS on the stack here...
Comment 34 Scott MacGregor 2006-02-24 13:23:51 PST
This solution isn't working on the 1.0.8 branch because we have two different script contexts. The JS url runs in one script context and editor disables JS in another. 

The patch works on the trunk because on the trunk, editor disables JS on a docshell and not the script context, so the script security manager code walks the docshell tree looking for parent docshells with JS disabled and finds this one. 

I suspect we'll want whatever bz ends up doing for 327078 AND the changes to convert editor to disable JS via a docshell instead of the script context, Bug 283149 (although I'm not sure why the patch that was checked in for that is marked as obsolete?) 
Comment 35 Scott MacGregor 2006-02-24 13:39:06 PST
(In reply to comment #33)
> Er... in that case, why does the patch for bug 327078 help?  I don't see an JS
> on the stack here...
> 

We call CanExecuteScrip several again a short time later once the iframe is created with a stack trace you will like more:

>	nsScriptSecurityManager::CanExecuteScripts() Line 1524	C++
 	nsDocument::IsScriptEnabled() Line 4670	C++
 	nsScriptLoader::DoProcessScriptElement() Line 370	C++
 	nsScriptLoader::ProcessScriptElement() Line 342	C++
 	nsHTMLScriptElement::MaybeProcessScript() Line 700	C++
 	nsHTMLScriptElement::DoneAddingChildren() Line 562	C++
 	HTMLContentSink::ProcessSCRIPTEndTag() Line 3924	C++
 	SinkContext::CloseContainer() Line 1406	C++
 	HTMLContentSink::CloseContainer() Line 2972	C++
 	CNavDTD::CloseContainer() Line 2677	C++
 	CNavDTD::HandleEndToken() Line 1586	C++
 	CNavDTD::HandleToken() Line 698	C++
 	CNavDTD::BuildModel() Line 331	C++
 	nsParser::BuildModel() Line 1752	C++
 	nsParser::ResumeParse() Line 1629	C++
 	nsParser::OnDataAvailable() Line 2266	C++
 	nsDocumentOpenInfo::OnDataAvailable() Line 361	C++
 	nsBaseChannel::OnDataAvailable() Line 597	C++
 	nsInputStreamPump::OnStateTransfer() Line 476	C++

Comment 36 Boris Zbarsky [:bz] 2006-02-24 14:28:14 PST
Right.  That's not it either.  I just reread the bug, and I guess the system principal doesn't come from JS on the stack but from the nodePrincipal of the <iframe> right?  that is, the about:blank document in question somehow has the system principal instead of an about:blank principal?
Comment 37 Scott MacGregor 2006-02-24 15:27:41 PST
Created attachment 213122 [details] [diff] [review]
additional patch for 1.0.8

Regardless of how bz's patch ends up looking, we'll also need this change to composer ported from part of Bug #283149 to disable JS on the docshell instead of the script context. This patch would be for 1.0.8 / 1.7.13 only.

This change plus what is currently in 327078 fixes the exploit on the 1.0.8 branch for me. I think the next step is to wait for a new patch in 327078.
Comment 38 georgi - hopefully not receiving bugspam 2006-02-24 22:43:44 PST
(In reply to comment #36)
> Right.  That's not it either.  I just reread the bug, and I guess the system
> principal doesn't come from JS on the stack but from the nodePrincipal of the
> <iframe> right?  that is, the about:blank document in question somehow has the
> system principal instead of an about:blank principal?
> 

i can't comment on the inner working of this, but heuristic shows:

on 1.0.7

reply: js, works but *NOT* with system principal (Components.classes not accessible)
forward inline: js works with system principal

on 1.5 and trunk

reply: js doesn't work
forward inline: js works with system principal





Comment 39 Boris Zbarsky [:bz] 2006-02-26 20:55:59 PST
OK, so I finally figured out how to use the testcase to reproduce this.

When I do "edit as new", the call into LoadFrame comes via ComposeStartup() calling InitEditor() which calls nsMsgCompose::BuildBodyMessageAndSignature, which calls into the editor.  In this case, mOwnerContent->GetNodePrincipal() is the about:blank principal.

I get the same stack, and the same principal, for "forward as inline" and "reply".

I guess I misunderstood comment 12 or something.  So ignore comment 36, please.


Comment 40 Scott MacGregor 2006-02-27 10:01:21 PST
Comment on attachment 213122 [details] [diff] [review]
additional patch for 1.0.8

this additional patch is needed for 1.0.8 and 1.7.13 in addition to bz's frame loader patch.
Comment 41 Scott MacGregor 2006-02-27 10:01:31 PST
Comment on attachment 213122 [details] [diff] [review]
additional patch for 1.0.8

this additional patch is needed for 1.0.8 and 1.7.13 in addition to bz's frame loader patch.
Comment 42 Daniel Veditz [:dveditz] 2006-02-27 10:15:19 PST
(In reply to comment #34)
> Bug 283149 (although I'm not sure why the patch that was checked in for that
> is marked as obsolete?) 

Because it's a timeless bug. It's checked in, therefore it's not needed anymore, therefore it's obsolete. Optimizes his searches for work he hasn't completed (especially his cross-tree cleanup bugs which often have multiple patches) at the expense of confusing the hell out of everyone else. He is always very clear in the bug comments, however, about exactly which patches are checked in including revision numbers.
Comment 43 Daniel Veditz [:dveditz] 2006-02-27 10:19:56 PST
Comment on attachment 213122 [details] [diff] [review]
additional patch for 1.0.8

yup, that helps.
Comment 44 Daniel Veditz [:dveditz] 2006-02-27 10:52:18 PST
(In reply to comment #42)
> Because it's a timeless bug.

I'm not faulting timeless, btw. Confusing 'til you figure it out, but he's just trying to deal with bugzilla limitations just as we are with the branch blocking-flag/fixed-keyword botch.
Comment 45 Scott MacGregor 2006-02-27 11:41:45 PST
bz has landed 327078 and I have now landed the patch in this bug on the 1.0.8 branch. Interestingly enough, 1.7.13 already had changes to nsEditingSession to disable JS on the docshell instead of the script context.

I'll add fixed 1.8.0.2 and fixed1.8.1 keywords when 327078 lands on those 2 branches.
Comment 46 Boris Zbarsky [:bz] 2006-02-27 11:46:42 PST
> 1.7.13 already had changes to nsEditingSession to disable JS on the docshell
> instead of the script context.

It does?  Where?  I don't see such changes at 
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/editor/composer/src/nsEditingSession.cpp&rev=MOZILLA_1_7_BRANCH&mark=169-178#168
Comment 47 Boris Zbarsky [:bz] 2006-02-27 11:47:31 PST
Comment on attachment 213122 [details] [diff] [review]
additional patch for 1.0.8

We need this on 1.7.13 as far as I can tell.
Comment 48 Scott MacGregor 2006-02-27 11:48:45 PST
Comment on attachment 213122 [details] [diff] [review]
additional patch for 1.0.8

I'm fixing it there. I was using the wrong tree :).
Comment 49 Scott MacGregor 2006-02-27 11:50:40 PST
fixed on the 1.7 branch too.
Comment 50 Daniel Veditz [:dveditz] 2006-02-27 20:11:06 PST
The fix from bug 327078 has landed on the 1.8 and 1.8.0 branches. The additional patch here isn't needed on 1.8.1 or 1.8.0.x because bug 283149 is fixed there.
Comment 51 georgi - hopefully not receiving bugspam 2006-02-27 23:35:14 PST
seems fixed on trunk suite cvs from this morning
Comment 52 Tracy Walker [:tracy] 2006-02-28 12:29:01 PST
verified on Linux Mozilla 1.7.13 2006-02-27
Comment 53 georgi - hopefully not receiving bugspam 2006-03-02 04:26:47 PST
it will be nasty if this can be exploited via mailto: uris
Comment 54 georgi - hopefully not receiving bugspam 2006-03-08 04:03:06 PST
a limited workaround for 1.5 is to use the text editor (uncheck 'compose message in html' from account setting).

in this case the exploit vector is only 'edit message as new' - forward and reply use the text version.
Comment 55 Bob Clary [:bc:] 2006-03-21 18:10:35 PST
verified fixed tb 1.5.0.2/winxp/20060308. No script execution when forwarding inline or when replying.
Comment 56 Jay Patel [:jay] 2006-04-05 16:18:14 PDT
v.fixed on Aviary branch with 4/5 build using both testcases.  I get the expected error blocking access in JSC.
Comment 57 georgi - hopefully not receiving bugspam 2006-04-13 12:32:43 PDT
> CVE-2006-0884

needless to say, in my humble opinion, CVE/mitre are not very good in sikiurity.
in addiotion, a mitre employee coauthored an RFC about "responsible disclosure" and was dumb enough to propose it to the ietf. the ietf replied something close to a word starting with F, containing U and ending in See-Key.

Comment 58 Daniel Veditz [:dveditz] 2006-04-14 09:36:23 PDT
> CVE-2006-0884
These are wanted/added by various linux distro people. Don't know why they can't just use our bugzilla link as a unique key for the problem. It seems somewhat arbitrary when a CVE number refers to an individual bug and when it refers to a group of bugs.

I do wish they'd keep them out of the way, though.
Comment 59 Wolfgang Rosenauer [:wolfiR] 2006-04-19 04:50:01 PDT
The JavaScript is still executed and opens a window (in case of the testcase) when you try to print the mail!
Comment 60 georgi - hopefully not receiving bugspam 2006-04-19 05:20:10 PDT
(In reply to comment #59)
> The JavaScript is still executed and opens a window (in case of the testcase)
> when you try to print the mail!
> 

this is Bug 328961

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