Closed Bug 41876 Opened 24 years ago Closed 24 years ago

Simple XUL file doesn't display, mime-type 'text/xul', over HTTP

Categories

(Core :: Security, defect, P3)

defect

Tracking

()

VERIFIED FIXED

People

(Reporter: jrgmorrison, Assigned: security-bugs)

References

()

Details

(Whiteboard: [nsbeta3+][HAVE FIX, need review])

Attachments

(2 files)

Overview Description:
  Crash loading a simple XUL file, mime-type 'text/xul', over network

Steps to Reproduce:
1) Load either of 
     http://bugzilla.mozilla.org/showattachment.cgi?attach_id=9400
     http://bugzilla.mozilla.org/showattachment.cgi?attach_id=9410
   which are attachments to bug #41126, and contain some very simple
   XUL content. 
                                                   
Actual Results:		       Expected Results:   
  Crashes		         Loads (they did load on 06/01)                
  
  
Reproducibility: always

Build Date & Platform Bug Found:
  2000060708 win32(win95)
  2000060710 linux(rh6.1)

Additional Builds and Platforms Tested On:

  DOES NOT CRASH on 2000060708 mac(os9)
   but instead gives an XML parsing error:

     "XML parsing error: not well-formed Line Number 32, Column 0"

  However, that location is at the end of file, past the final 
  closing tag '</window>'. (These are well-formed files, and did load
  on May31/Jun01 when I last checked them). 

Additional Information:

  bug #41126 is entirely unrelated to this crash; that bug is a subtle
  bug in JS dealing with enumerating the DOM properties of XBL elements;
  The actual XUL content of the files are trivial.
  
  linux stack trace (build from midnight PDT last night)

(gdb) bt
#0  0x405a2117 in memcpy (dstpp=0x87e33f0, srcpp=0x0, len=695)
    at ../sysdeps/generic/memcpy.c:55
#1  0x400f7a1b in nsByteArrayInputStream::Read (this=0x8700000, 
    aBuffer=0x87e33f0 "<?xml version=\"1.0\"?> \n<?xml-stylesheet 
href=\"chrome://global/skin/\" type=\"text/css\"?>\n<window 
xmlns:html=\"http://www.w3.org/TR/REC-html40\"\n        
xmlns=\"http://www.mozilla.org/keymaster/gatekeeper/t"..., aCount=695, 
aNumRead=0xbffff330) at nsByteArrayInputStream.cpp:72
#2  0x40e2b0d5 in InterceptStreamListener::Read (this=0x86ab150, 
    buf=0x87e33f0 "<?xml version=\"1.0\"?> \n<?xml-stylesheet 
href=\"chrome://global/skin/\" type=\"text/css\"?>\n<window 
xmlns:html=\"http://www.w3.org/TR/REC-html40\"\n        
xmlns=\"http://www.mozilla.org/keymaster/gatekeeper/t"..., 
    count=695, aActualBytes=0xbffff330) at nsCachedNetData.cpp:1177
#3  0x41973f9e in nsParser::OnDataAvailable (this=0x85b9dc8, 
    channel=0x8958ee8, aContext=0x0, pIStream=0x86ab154, sourceOffset=0, 
    aLength=695) at nsParser.cpp:1886
#4  0x40f1fe09 in nsDocumentOpenInfo::OnDataAvailable (this=0x8959050, 
    aChannel=0x8958ee8, aCtxt=0x0, inStr=0x86ab154, sourceOffset=0, count=695)
    at nsURILoader.cpp:210
#5  0x40e5a60c in nsHTTPFinalListener::OnDataAvailable (this=0x8958e08, 
    aChannel=0x8958ee8, aContext=0x0, aStream=0x86ab154, aSourceOffset=0, 
    aCount=695) at nsHTTPResponseListener.cpp:1216
#6  0x40e2b259 in InterceptStreamListener::OnDataAvailable (this=0x86ab150, 
    channel=0x8958ee8, ctxt=0x0, inStr=0x8700000, sourceOffset=0, count=695)
    at nsCachedNetData.cpp:1164
#7  0x40e2272b in nsHTTPChunkConv::OnDataAvailable (this=0x87e0940, 
    aChannel=0x8958ee8, aContext=0x0, iStr=0x894d5bc, aSourceOffset=0, 
    aCount=5) at nsHTTPChunkConv.cpp:208
#8  0x40e58b91 in nsHTTPServerListener::OnDataAvailable (this=0x8906e38, 
    channel=0x86d334c, context=0x8958ee8, i_pStream=0x894d5bc, 
    i_SourceOffset=897, i_Length=5) at nsHTTPResponseListener.cpp:540
#9  0x40df591f in nsOnDataAvailableEvent::HandleEvent (this=0x895f000)
    at nsAsyncStreamListener.cpp:406
#10 0x40df4ba7 in nsStreamListenerEvent::HandlePLEvent (aEvent=0x895f028)
    at nsAsyncStreamListener.cpp:97
#11 0x4010ee8e in PL_HandleEvent (self=0x895f028) at plevent.c:575
#12 0x4010ed3c in PL_ProcessPendingEvents (self=0x813bfb0) at plevent.c:520
#13 0x40110ac9 in nsEventQueueImpl::ProcessPendingEvents (this=0x8126760)
    at nsEventQueue.cpp:356
#14 0x40a371c4 in event_processor_callback (data=0x8126760, source=9, 
    condition=GDK_INPUT_READ) at nsAppShell.cpp:158
#15 0x40a36dff in our_gdk_io_invoke (source=0x8214d78, condition=G_IO_IN, 
    data=0x8214d68) at nsAppShell.cpp:58
#16 0x40bfa52a in g_io_unix_dispatch () from /usr/lib/libglib-1.2.so.0
#17 0x40bfbbe6 in g_main_dispatch () from /usr/lib/libglib-1.2.so.0
#18 0x40bfc1a1 in g_main_iterate () from /usr/lib/libglib-1.2.so.0
#19 0x40bfc341 in g_main_run () from /usr/lib/libglib-1.2.so.0
#20 0x40b23209 in gtk_main () from /usr/lib/libgtk-1.2.so.0
#21 0x40a378ba in nsAppShell::Run (this=0x8117c60) at nsAppShell.cpp:334
#22 0x4070cff4 in nsAppShellService::Run (this=0x81105a8)
    at nsAppShellService.cpp:386
#23 0x80533e9 in main1 (argc=1, argv=0xbffffab4, nativeApp=0x0)
    at nsAppRunner.cpp:906
#24 0x8053ace in main (argc=1, argv=0xbffffab4) at nsAppRunner.cpp:1092
(gdb)
... or maybe this isn't Parser, per se.
Blocks: 41878
Some other crashers in nsByteArratInputStream::Read are:
bug 27507 rickg   Crash instead of blank page for malformed xhtml page 
bug 40893 gagan   always crash in this URL 
bug 41248 gagan   (topcrash) Crashes at MSVCRT.dll + 0x1648 (0x78001648)
Keywords: crash
Forwarding to gagan (this appears to be another netwerking stream bug like the 
one we saw last week). Another note: I didn't crash with either of the attached 
testcases. The pages loaded just fine, and rendered correctly.
Assignee: rickg → gagan
Let me quickly look into it. Reassigning to myself
Assignee: gagan → ruslan
The crash was due to the fact that the converter in necko wasn't switching the 
state correctly after the upstream listener returned the failure code. I'm 
going to check in the fix for the converter, but the real bug is somewhere in 
parser/layout - as result, the browser will not crash, but will not renter the 
page either. I can work on it a bit more of feel free to look into it as well.
Status: NEW → ASSIGNED
Back to rickg. Looks like nsParser to me. The crash is gone now, as the stream 
converter will handle the error code properly. The page just wound't display.
Assignee: ruslan → rickg
Status: ASSIGNED → NEW
removing crash from keyword field. I'll see why the text doesn't display.
Status: NEW → ASSIGNED
Keywords: crash
Over to harishd.
To my list.
Assignee: rickg → harishd
Status: ASSIGNED → NEW
Adding crash keyword
Keywords: crash
Changing the definition and removing crash keyword as it doesn't crash anymore.
Keywords: crash
Summary: Crash loading a simple XUL file, mime-type 'text/xul', over HTTP → Simple XUL file doesn't display, mime-type 'text/xul', over HTTP
Sounds like a bug in
nsScriptSecurityManager::CheckLoadURI(nsIURI *aFromURI, nsIURI *aURI,
                                      PRBool aDisallowFromMail);

Over to mstoltz@netscape.com
Assignee: harishd → mstoltz
Component: Parser → Security: General
Remote XUL references a chrome:// URL in a stylesheet tag, where the security
check is currently preventing it from loading. I added the security check to
stylesheet tags to fix bug 16858, but it has caused some unforseen problems. I
need to figure out when chrome: URLs are safe to load and when they're not, and
rewrite this policy.
Status: NEW → ASSIGNED
Target Milestone: --- → M17
Adding nsbeta3, need to resolve this and related issues for beta3.
Keywords: nsbeta3
[nsbeta3-] with beard. (1) Disallowing loading of local URL access in remote
content seems like the safe route to take. (2) Is there a legitimate case where
remote content cannot (for some reason) just have its own copy of the style sheet?
Whiteboard: [nsbeta3-]
Perhaps this should be reconsidered -- without this fixed, there is
absolutely no possibility of using remote XUL widgets in any way. Aside from
the interest in this shown by external parties, the loss of the ability to
load remote XUL has also inhibited testing for myself and for the
developers.

This fix would allow referencing of local style information, but it is style
information only. (I'm not sure what you meant by "loading" in the above
note).  Checks are already in place so that no scripts _of any kind_ can be
executed from the skin directory.

While it is possible to reference style sheet information for XUL over the
web, it is not practical to deliver these complete skins on an HTTP
server. Aside from the effort required to fork a new version of each skin
for HTTP use, these skins (CSS and images) would not pre-cached
locally. Even then, there would be no way to deliver the correct set of
widget styles that correspond to a particular user's selected theme.
Ok, here's my opinion.  Chrome URLs should always be safe to load by any one at 
any time from any page, remote or local.  I understand that we're trying to 
prevent potential security holes, but so far nobody has pointed out any security 
flaw from allowing chrome URLs to be loaded by remote XUL.

(1) Remote XUL should be allowed to load the user's current skin.  That means 
chrome URLs that point to stylesheets need to be allowed.
(2) Remote XUL should be allowed to load XBL widgets installed as downloaded 
packages.  So chrome XBL files should be ok.
(3) Remote XUL should be able to make use of helper overlays like 
dialogOVerlay.xul that set up dialogs.  This means chrome XUL files should be 
ok.
(4) Remote XUL should be allowed to make use of our JS libraries (like the drag 
and drop and file JS libraries).  This means chrome JS files should be ok.

Basically remote XUL has been neutered by this change, and until someone gives 
me an actual example of a security hole exposed by cutting off chrome from 
remote XUL, I'm going to keep screaming for this to be fixed.
cc:ing jar for additional input on security implications. Questions (pardon my 
ignorance):
1) Can't XUL files contain inline JavaScript code or links to local JS files?
2) What inherent limitations are there on the content of chrome files that 
prevent them from becoming a security vulnerability?
3) We can be confident about Netscape-created chrome files not containing 
deliberately malicious stuff. But what about the case where a site gets a user 
to download a seemingly-safe skin which contains some JS code that is then 
somehow misused?
4) One of the basic principles of our security architecture has been preventing 
remote sites from loading local code or data (without being a signed script 
that's requested & received user permission), right? I'm concerned about 
creating any exceptions to this rule.

Pardon me if these are non-issues or I'm clueless; my ignorance about chrome is 
vast. But I'm really, really nervous about us deciding that we're smart enough 
to be sure that creating an exception to the can't-load-local-code rule is not 
going to enable exploits, especially unforeseen ones. ;->
The four cases I cited should work.  Block off everything else... but the four 
cases above should work.
Hyatt and I have agreed that his statement "Chrome URLs should always be safe to 
load by any one at any time from any page, remote or local" is not correct and is 
not our goal. If we make sure that any content (scripts, especially) included by 
a page run with the principal of that page, then loading CSS, JS, XUL overlays, 
etc. from chrome on a remote XUL page is probably all right. We need to verify 
this assumption. 
I don't know if this is relevant to this particular discussion, but it seems to 
me that the code at this line: 
http://lxr.mozilla.org/seamonkey/source/rdf/chrome/src/nsChromeProtocolHandler.
cpp#708 should be dependent on the principal of the underlying channel. It 
should not get blanket system principal privileges.
Warren,
   We do have to assign the system principal here, at least to chrome XUL 
documents, or chrome won't have access to xpconnect, and nothing will work. 
You're right in that we don't have to assign the system principal to everything 
loaded through chrome; I'm looking at how to tighten this up. The principal of 
the underlying channel won't work for chrome, since that would mean a "file://" 
principal, which is NOT the system principal and is not automatically privileged. 
There is a distinction to be made between chrome and other local files. 
I think you should do this:

  nsCOMPtr<nsIFile> localFile;
  rv = result->GetLocalFile(getter_AddRefs(localFile));
  if (NS_SUCCEEDED(rv) && localFile) {
    result->SetOwner(systemPrincipal);
  }
  else {
    // dumb-down principal of non-local-file channels (or leave it alone)
  }
Warren,
  Isn't chrome always going to be a local file? Why do we need to check? I think 
chrome is always local, though I've filed bug 48083 to make sure of this. We 
could be even more restrictive in the chrome protocol handler, and give the 
system principal only to xul files loaded from chrome.
You'll have to ask Hyatt that, but I've heard rumblings that people want to 
download chrome (eventually).
I have learned from Hyatt that this is indeed important; remote XUL is broken
and it's a feature we want. I think I can fix this without opening any holes.
Please reconsider for nsbeta3.
Whiteboard: [nsbeta3-]
Marking [nsbeta3+] with proviso that only 4 cases mentioned by hyatt will be 
allowed. Does this bug still trigger a crash on any of the platforms?
Whiteboard: [nsbeta3+]
I've made it so that anyone, anywhere can load chrome URLs. After talking to
Hyatt, I think this is safe, since chrome is sandboxed and privileges applied to
chrome should not be able to be 'stolen' by remote content. For an extra measure
of security, I changed the chrome protocol handler so that it no longer assigns
system principals indiscriminately. Instead, it gives the system principal only
to XUL files. I've tested this a bit (precheckin tests and simple remote XUL)
and nothing seems to break, but I don't know what else to test.

Two patches to follow, one to caps for the regression fix, one to rdf/chrome for
the tightening of system principal granting.
Whiteboard: [nsbeta3+] → [nsbeta3+][HAVE FIX, need review]
Fix is in.
Status: ASSIGNED → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
I don't knwo our security architecture, but it seems to me that this is a rather
drastic and very dangerous change. mstoltz, can you please post to .security,
ideally with some background (or references to such docs), so we can discuss
this there?
Ben, 
   I discussed this with David Hyatt, and we decided it was safe. Here are some 
invariants for chrome (you might want to verify these for yourself in the code):
1. chrome:// URLs can only resolve to files in the chrome directory.
2. An attacker cannot modify the chrome directory.
3. Only XUL files loaded through chrome are given system principals.
3a. If a remote document, xul or html, loaded over http, ftp, etc, includes a 
chrome:// URL (as a stylesheet, overlay, XBL, image etc.), the document and all 
of its included content retains the principal of the remote document; none of it 
is given the system principal or any other.

The main danger in a remote site's being able to load chrome is that chrome XUL 
documents have the all-powerful system principal. The above invariants prove that 
an attacker cannot make use of this system principal. There are other issues in 
remote sites loading any local content in well-known locations, such as the 
Haselton attack (reading prefs.js), and the ability to see if a local file 
exists. There is minimal leakage of privacy here, as an attacker can view the 
contents of your chrome directory and determine your current skin, but I have 
been told this is by design; that the chrome directory is meant to have a well-
known structure and be essentially world-readable. That's a tangential issue.
   Does this ease some of your concern? Can you think of any other ways to 
exploit access to chrome? If you want to start a thread on n.p.m.security, be my 
guest; I'll respond there.
Hmmm... if a site can run arbitrary XUL, can they put up more than picture
perfect replicas of password entry dialogs? Do they get to use the exact skin
and blend in perfectly 'cause we helped? There have been enough spoofing attacks
of this sort, but at least the attacker had to work really hard.  This might
make it really easy.  Do we craft the dialogs so that an attacker couldn't pull
the entered values from the dialogs?

I don't think we can stop spoofing attacks, since folks can draw on a graphics
context any set of pixels we want.  It seems a shame to make it too darn easy to
get all the pixels just right. Is there any thing we can do in the crafting of
password dialogs to make them harder to use directly?  Is there *any* XUL that
is restricted?
mstoltz, what about chrome js files?

So, if a remote site can load an XUL file with system principal, can't an
attacker access this chrome and make it do what the he wants? E.g. open a new
navigator window, trigger the file open dialog, fill it in, trigger OK and then
read the content of the file from the navigator window? If that doesn't work,
are you *sure* that nothing like this (maybe in a more mild form) can happen?

If I have to choose between remote XUL or higher security (i.e. smaller
/likelyness/ of exploits), I don't have to think twice.

mstoltz, I am not really qualified to make the introducing post to .security.
It's the static scope of the calling script that's used to determine the
principals, and thereby the privileges, of executing script, so I don't think
there's a problem with unprivileged script co-opting the chrome scripts.  You
can see the code in question in caps/src/nsScriptSecurityManager.
Given that CSS2 supports native system colors and fonts, a remote HTML page can 
now look just like a native dialog.  This isn't an issue that's confined to XUL, 
and it's an issue regardless of whether you're using our chrome URLs, i.e., you 
don't need to access chrome URLs to look as if you're a native dialog.  You 
don't even need XUL to look like a native dialog.  You could do it all with 
remote HTML.

So yes, you're pointing out a spoofing attack, but the attack would exist 
regardless of whether or not you accessed local chrome URLs.  danm has a bug 
on XUL spoofing and was planning on adding [REMOTE/UNTRUSTED XUL] or somesuch to 
remote XUL titlebars to solve the problem.

Seems reasonable. As Jar points out, marking a window as untrusted doesn't 
prevent spoofing, but it does make it more difficult. An attacker should not be 
able to use the user's chrome to create a perfect replica of a password dialog in 
the user's current skin. 
But the attacker can already create a perfect native-looking dialog without 
referring to the user's skin, so I fail to see how being able to use the skin is 
any worse... 

Another point... our skin files are all well-known and published... someone 
using remote XUL could simply copy all of them to their remote server and then 
have a perfect look from some skin...

The point is that most users will stick with the default, and remote XUL could 
just style itself to look like that default... or to look native... you can't 
stop this... HTML could also style itself to look like our default...

We were figuring that we'd just put some visual indicator on the window to 
indicate that it is untrusted and leave it at that... 

I think we're in violent agreement. I was just agreeing with Jar's comment that 
we can't stop spoofing but that doesn't mean we have to make it really simple. 
Marking windows as untrusted will accomplish this.
Our general use of dialogs popping up whenever they feel like (seemingly) to ask 
for passwords tends to be problematic.  We do it... but it makes spoofing into a 
very fun sport :-(.

Imagine if I wanted to help the situation, and I put (as an example) a picture 
of my daughter in as a background for any/all login dialogs on *my* copy of the 
browser.  For the sake of argument, lets assume I pick an arbitrary file on my 
hard disk to act as this background (i.e., an attacker can't guess the gif or 
location).  The question is: Can I set this up in such a way that any/all 
attackers can't merely refer to  my password dialog, and get a perfect replica?  
Yes it is true that if I leave everything at the default value, then I'd have no 
visual safety (defaults are published and known), but would it be possible to do 
anything like this?

Simply put, I think that it would sure be nice to have some privacy potential on 
the local machine with regard to the UI in use (especially for password entry 
dialogs).  I'm a little scared that this bug suggests that anything that the app 
can present, an attacker can also present (with assurances of being a dead 
match). (...but maybe I'm not groking this bug??).

Maybe there is some great value here, and that value needs to be traded off 
against privacy.  I don't really know.  I'd like to understand the cost/value 
proposition.

I'm not sure that there is a great value for security... but I'm not sure (yet) 
of the value gained that might preclude setups such as described above.

Thanks,

Jim
They can't get an exact match if we have a visual indicator that the window is 
untrusted, which is what we're proposing to do.... are you worried that text in 
the window titlebar is an insufficient visual indicator?
" - [JavaScript Application]" following the nominal window title (by default,
the host part of the URI) was good enough to frustrate spoofing in the titlebar
for 4.x.  I say it's good enough for Mozilla.

/be
Permitting some sort of shared secret to authenticate our dialogs would
certainly be an option, and perhaps a good one for those who want something
stronger than title-bar suffixes.  As far as malicious content being able to
steal that secret, I don't think that unprivileged content can access chrome
data, due to our sandboxing.

Protecting against privileged content isn't feasible or interesting to us, I
assert.
Mike, you're not reading very carefully. The issue we're discussing _is_ allowing 
untrusted content to access chrome data. This seemed insecure to me as well, but 
apparently it was in the plans all along, as remote XUL is useless without access 
to local chrome (or so Hyatt has told me), and remote XUL is a feature we want to 
ship.
When you say ``access to local chrome'', do you mean:
 - the ability to specify chrome: URLs from web content, such as style sheets or
images, or
 - the capability for remotely-loaded content to access the DOM of chrome
content (as created by our password dialogs, for example), through parent or
traversal or window enumeration
?

I thought we were talking about the former -- and this bug's summary indicates a
different thing entirely, which is simply the ability to load text/xul from the
network -- and not the latter.  Why bother spoofing our password dialogs when
you can just wait for Mozilla to create one and snoop in it?
Sidenote:
> This seemed insecure to me as well, but [...]
> remote XUL is a feature we want to ship.

This reasoning is exactly what concerns me. And nothing said here destroyed my
concerns so far.
Ben: I think we may share the same concerns, and jar as well[*], but I'm not
certain.

Can you enumerate those concerns for us, perhaps in the newsgroup?

[*] one uncharacteristic but frequent point of jar-shaver agreement is security
rigor, I recall. =)
> Can you enumerate those concerns for us, perhaps in the newsgroup?

Will try.
Posted "Should anonymous content be able to open chrome?"
<news://news.mozilla.org/9AC352C.2080803@bucksch.org> to .security.

What I just saw:
Additional Comments From ekrock@netscape.com 2000-08-07 14:31:
> (4) Remote XUL should be allowed to make use of our JS libraries (like the
> drag and drop and file JS libraries).

Why does remote chrome need file access???
To take advantage of our JS libraries.  This is ok, since the JS executes in the 
context of the remote doc, so if it tries to do anything nasty, it would be 
denied.
It is beyond scary when remote XUL can access the drag/drop features of a window 
system.  The drag/drop API on most systems are very hard to control from a 
security perspective.  Most API allow the window to *decide* it wants to read or 
write the API. The presumtion is that the user *asks* the window to read from 
the clipboard (or drag source), and then the window takes the action.  Alas, 
rouge windows can silently snoop from the clipboard, and/or observe dragged 
data, even though the user never requested any such activity.

I expect it would be a very non-standard API that would be needed to allow such 
access from untrusted code.  The problem is that the trusted infrastructure 
would have to decide that a window was given a "cut" or "paste" etc. request, 
and then  that trusted infrastructure would have to access the clipboard.  Alas, 
most requests from users (events sent to windows) can be simulated by the 
window, and this is commonly done to translate keys and clicks to events 
specified by the window :-/.

Bottom line: If drag/drop is supported, then I suspect that we will have 
signifcant security problems, unless there is an API design with INCREDIBLE 
focus on security (not just a small fix effort).
It cannot access the drag and drop API, since the drag and drop API is walled 
off in this release.  As it happens, we've walled off practically everything in 
this release, so accessing the local JS libraries is next to useless.  Most of 
these libraries end up falling into XPConnect, which we've essentially cut off 
from remote chrome.

However the pure JS routines can still be accessed, and those are safe.  Anyway, 
I'm tired of typing in this bug.  Can we have a meeting if there are still 
concerns?
> Can we have a meeting if there are still concerns?

If you do, please announce it on .security and make it accessible (via dial-in)
to people outside Netscape. An alternative the .security newsgroup *g*. This
topic is too important for all of us to discuss it behind closed doors.
No longer blocks: 41878
reassigned ca contact to lchiang
QA Contact: janc → lchiang
john morrison - can you verify this for me?  I don't know why Jan assigned this
for me.
Okey-dokey. (I already know this generally works, will look more this evening).
QA Contact: lchiang → jrgm
No more crash loading the two test files on win/mac/linux(2000091408m18).Marking 
VERIFIED.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: