Closed Bug 319858 Opened 19 years ago Closed 18 years ago

javascript execution when forwarding or replying (CVE-2006-0884)

Categories

(MailNews Core :: Composition, defect)

1.7 Branch
x86
Linux
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: guninski, Assigned: mscott)

References

Details

(Keywords: fixed1.8.1, verified1.7.13, verified1.8.0.2, Whiteboard: [sg:critical] mail bug [qa:verified-tb-1802])

Attachments

(3 files)

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
Attached file local folder
local folder
Need to investigate the context and make sure chrome execution is not possible.
Assignee: dveditz → mscott
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: blocking1.8.0.1+
Flags: blocking-aviary1.0.8+
Whiteboard: [sg:high]
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...
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.
Flags: blocking-thunderbird2+
Whiteboard: [sg:high] → [sg:high] mail bug
Thunderbird just released its 1.5 and given the timing is going to sync up at 1.5.0.2
Flags: blocking1.8.0.1+ → blocking1.8.0.2+
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. 
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. 


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.

Too late for 1.0.8
Flags: blocking-aviary1.0.9?
Flags: blocking-aviary1.0.8-
Flags: blocking-aviary1.0.8+
This issue has been independently reported at http://sysdream.com/article.php?story_id=230&section_id=78

-> critical based on comment 7
Whiteboard: [sg:high] mail bug → [sg:critical] mail bug
Scott, what caller is passing a system principal to CanExecuteScripts, exactly?
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.  
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.
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.
Depends on: 327078
when selecting "reply" is the principal still system?

can't access Components.classes in this case and get js error.
this is related to Bug 319859
<iframe src='file:///etc/passwd'> works when "forward as inline" on trunk and 1.5
When I replied to the test message, the principal was still not the system principal, and the JS failed to execute. 
Flags: blocking-aviary1.0.8- → blocking-aviary1.0.8+
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.
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?
(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. 

does the patch fix accessing /etc/passwd in editor?

if not, i shall file a new bug.
(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.
> 
> 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?


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.
(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).
(In reply to comment #27)

ok, will file a new bug.

indeed, 1.0.7 and 1.0.8 seem safe from iframe.
(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.
"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.
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

Er... in that case, why does the patch for bug 327078 help?  I don't see an JS on the stack here...
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?) 
(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++

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?
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.
(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





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 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.
Attachment #213122 - Flags: approval-aviary1.0.8?
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.
(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 on attachment 213122 [details] [diff] [review]
additional patch for 1.0.8

yup, that helps.
Attachment #213122 - Flags: approval-aviary1.0.8? → approval-aviary1.0.8+
(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.
Component: Security → MailNews: Composition
Product: Thunderbird → Core
Version: unspecified → 1.7 Branch
Flags: blocking1.7.13+
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.
Status: NEW → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
> 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 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.
Attachment #213122 - Flags: approval1.7.13?
Comment on attachment 213122 [details] [diff] [review]
additional patch for 1.0.8

I'm fixing it there. I was using the wrong tree :).
Attachment #213122 - Flags: approval1.7.13? → approval1.7.13+
fixed on the 1.7 branch too.
Keywords: fixed1.7.13
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.
seems fixed on trunk suite cvs from this morning
verified on Linux Mozilla 1.7.13 2006-02-27
it will be nasty if this can be exploited via mailto: uris
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.
verified fixed tb 1.5.0.2/winxp/20060308. No script execution when forwarding inline or when replying.
Whiteboard: [sg:critical] mail bug → [sg:critical] mail bug [qa:verified-tb-1802]
v.fixed on Aviary branch with 4/5 build using both testcases.  I get the expected error blocking access in JSC.
Summary: javascript execution when forwarding or replying → CVE-2006-0884 javascript execution when forwarding or replying
> 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.

> 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.
Summary: CVE-2006-0884 javascript execution when forwarding or replying → javascript execution when forwarding or replying (CVE-2006-0884)
The JavaScript is still executed and opens a window (in case of the testcase) when you try to print the mail!
(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
Flags: blocking-aviary1.0.9?
Group: security
Product: Core → MailNews Core
You need to log in before you can comment on or make changes to this bug.