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)
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)
1.77 KB,
text/plain
|
Details | |
759 bytes,
text/plain
|
Details | |
1003 bytes,
patch
|
dveditz
:
approval-aviary1.0.8+
mscott
:
approval1.7.13+
|
Details | Diff | Splinter Review |
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
Reporter | ||
Comment 1•19 years ago
|
||
local folder
Comment 2•19 years ago
|
||
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]
Comment 3•19 years ago
|
||
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...
Reporter | ||
Comment 4•19 years ago
|
||
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.
Updated•18 years ago
|
Flags: blocking-thunderbird2+
Whiteboard: [sg:high] → [sg:high] mail bug
Comment 5•18 years ago
|
||
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+
Assignee | ||
Comment 6•18 years ago
|
||
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.
Assignee | ||
Comment 7•18 years ago
|
||
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.
Assignee | ||
Comment 8•18 years ago
|
||
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•18 years ago
|
||
Too late for 1.0.8
Flags: blocking-aviary1.0.9?
Flags: blocking-aviary1.0.8-
Flags: blocking-aviary1.0.8+
Comment 10•18 years ago
|
||
This issue has been independently reported at http://sysdream.com/article.php?story_id=230§ion_id=78 -> critical based on comment 7
Whiteboard: [sg:high] mail bug → [sg:critical] mail bug
Comment 11•18 years ago
|
||
Scott, what caller is passing a system principal to CanExecuteScripts, exactly?
Assignee | ||
Comment 12•18 years ago
|
||
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•18 years ago
|
||
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.
Assignee | ||
Comment 14•18 years ago
|
||
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
Reporter | ||
Comment 15•18 years ago
|
||
when selecting "reply" is the principal still system? can't access Components.classes in this case and get js error.
Reporter | ||
Comment 16•18 years ago
|
||
this is related to Bug 319859
Reporter | ||
Comment 17•18 years ago
|
||
<iframe src='file:///etc/passwd'> works when "forward as inline" on trunk and 1.5
Assignee | ||
Comment 18•18 years ago
|
||
When I replied to the test message, the principal was still not the system principal, and the JS failed to execute.
Updated•18 years ago
|
Flags: blocking-aviary1.0.8- → blocking-aviary1.0.8+
Assignee | ||
Comment 19•18 years ago
|
||
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•18 years ago
|
||
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?
Assignee | ||
Comment 21•18 years ago
|
||
(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.
Reporter | ||
Comment 22•18 years ago
|
||
Reporter | ||
Comment 23•18 years ago
|
||
does the patch fix accessing /etc/passwd in editor? if not, i shall file a new bug.
Comment 24•18 years ago
|
||
(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.
Reporter | ||
Comment 25•18 years ago
|
||
>
> 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?
Reporter | ||
Comment 26•18 years ago
|
||
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•18 years ago
|
||
(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).
Reporter | ||
Comment 28•18 years ago
|
||
(In reply to comment #27) ok, will file a new bug. indeed, 1.0.7 and 1.0.8 seem safe from iframe.
Reporter | ||
Comment 29•18 years ago
|
||
filed Bug 328445
Comment 30•18 years ago
|
||
(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.
Reporter | ||
Comment 31•18 years ago
|
||
"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.
Assignee | ||
Comment 32•18 years ago
|
||
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•18 years ago
|
||
Er... in that case, why does the patch for bug 327078 help? I don't see an JS on the stack here...
Assignee | ||
Comment 34•18 years ago
|
||
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?)
Assignee | ||
Comment 35•18 years ago
|
||
(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•18 years ago
|
||
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?
Assignee | ||
Comment 37•18 years ago
|
||
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.
Reporter | ||
Comment 38•18 years ago
|
||
(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•18 years ago
|
||
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.
Assignee | ||
Comment 40•18 years ago
|
||
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?
Assignee | ||
Comment 41•18 years ago
|
||
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•18 years ago
|
||
(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•18 years ago
|
||
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+
Comment 44•18 years ago
|
||
(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.
Updated•18 years ago
|
Component: Security → MailNews: Composition
Product: Thunderbird → Core
Version: unspecified → 1.7 Branch
Updated•18 years ago
|
Flags: blocking1.7.13+
Assignee | ||
Comment 45•18 years ago
|
||
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•18 years ago
|
||
> 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•18 years ago
|
||
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?
Assignee | ||
Comment 48•18 years ago
|
||
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+
Comment 50•18 years ago
|
||
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.
Keywords: fixed1.8.0.2,
fixed1.8.1
Reporter | ||
Comment 51•18 years ago
|
||
seems fixed on trunk suite cvs from this morning
Comment 52•18 years ago
|
||
verified on Linux Mozilla 1.7.13 2006-02-27
Keywords: fixed1.7.13 → verified1.7.13
Reporter | ||
Comment 53•18 years ago
|
||
it will be nasty if this can be exploited via mailto: uris
Updated•18 years ago
|
Keywords: fixed1.8.0.2 → verified1.8.0.2
Reporter | ||
Comment 54•18 years ago
|
||
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•18 years ago
|
||
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]
Comment 56•18 years ago
|
||
v.fixed on Aviary branch with 4/5 build using both testcases. I get the expected error blocking access in JSC.
Keywords: fixed-aviary1.0.8 → verified-aviary1.0.8
Updated•18 years ago
|
Summary: javascript execution when forwarding or replying → CVE-2006-0884 javascript execution when forwarding or replying
Reporter | ||
Comment 57•18 years ago
|
||
> 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•18 years ago
|
||
> 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)
Comment 59•18 years ago
|
||
The JavaScript is still executed and opens a window (in case of the testcase) when you try to print the mail!
Reporter | ||
Comment 60•18 years ago
|
||
(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
Updated•18 years ago
|
Flags: blocking-aviary1.0.9?
Updated•18 years ago
|
Group: security
Updated•16 years ago
|
Product: Core → MailNews Core
You need to log in
before you can comment on or make changes to this bug.
Description
•