User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.7) Gecko/20050414 Firefox/1.0.3 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.7) Gecko/20050414 Firefox/1.0.3 The XUL error pages are a chrome: url. While the error page itself isn't privileged, it can redirect to privileged chrome files which allows to execute arbitrary code. When the XUL error page is embedded in an iframe it is only protected from the parent content by the cross site scripting restrictions. This automaticly makes all cross site scripting bugs (like the currently unpatched 290982) automaticly an arbitrary code execution bug. Reproducible: Always Steps to Reproduce: 1. Open http://bugzilla:Jq6w2Rt@www.mikx.de/firepaging/ 2. Follow instructions Since XUL error pages are going to be default in 1.1 (afaik) it should be made sure, that they can not be used for privilege elevation attacks under any condition. Cross site scripting is only one option. If the propagation of the drag and drop event ever changes in a way that allows to drop a link to an embedded frame instead of the top document, a single drag operation (e.g. faked scrolling) would be enough to exploit the error page.

Further testings seem to show that this bug got introduced with 1.0.3. I can repro on 1.0.3 Win32 and 1.0.3 Mac. But not on 0.9.2 / 1.0 / 1.0.2 Win32 or 1.0 / 1.0.2 Mac.

Probably don't need for 1.0.x since this is not the default setting (and no user-facing UI to switch it). Definitely a problem on the trunk. Uses view-source:javascript: so might get fixed anyway.

Not blocking branch release (feature not on, view-source:javascript defanged).

Taking: I'm hoping to use a separate protocol "-x-moz-errorpage:" to avoid privilege escalation.

An URL scheme cannot begin with a hyphen. It must begin with an ASCII letter.

Aren't there some about: pages that run with chrome privs? about:plugins? We currently prevent sites from loading a file:// URL into an <iframe>, so why don't we similarly restrict chrome:// and resource://? Why do we need another URL protocol for error pages?

We do restrict chrome from being loaded -- assuming someone doesn't find a way around it which they occassionally have in the past. We think we've plugged all the holes, but have we? Merely using a different protocol isn't going to help. If the template/code file is in the chrome jar as it is now then the attacker wouldn't use the new protocol, they'd still be able to open the file using the chrome: protocol. The current error pages accept arguments on the chrome: url which makes it particularly prone to external attack. If it required arguments[] like all the other dialogs it'd be a lot safer. Unless you're talking about using a new protocol, and then generating the page completely dynamically with no .xul found in a .jar anywhere.

Bumping to b4, as there are not known attack vectors.

I discovered all sorts of nasty corners or our security system while doing this patch: the bit about originalURI in nsDocument was particularly annoying, as I don't understand why we can't use originalURI consistently and need that protocol whitelist.

Note: this patch should also remove docshell/resources/content/jar.mn and docshell/resources/content/netError.js

why are you removing the pref? (are embeddors now supposed to override the protocol handler impl if they want their own error page? (which may actually be trivial for some))

If we really want the pref, it cannot be in nsDocshell, it must be in nsErrorPageProtocolHandler.cpp. *However*, the pref *must not* point to a chrome URL, or else the channel gets the system principal anyway.

bsmedberg: why is a new protocol scheme the best solution here?

Darin, the protocol change is IMO the best solution because 1) we don't want to give the page system privs 2) we always want the page to be able to run script Plus it seems like a fairly simple solution. I'm open to other solutions but I can't think of any offhand.

How about "about:neterror?params" nsScriptSecurityManager.cpp already allows about: pages to run script when script is enabled. They are not given system privs by default (AFAIK), and it is really easy to define a new about: page via the about redirector. Why not this solution?

I meant "when script is _disabled_" nsAboutRedirector.cpp allows you to specify the about pages that have chrome privs and those that do not.

if you use about:foo then there must be a pref. embeddors (epiphany) want to override the error page.

> if you use about:foo then there must be a pref. embeddors (epiphany) want to > override the error page. I don't understand. The about:foo would be redirector to some file, either in a chrome package or in the res folder. Epiphany could just override that file instead of the URL used to indirectly load it.

I'm not sure I understand what you mean, are you suggesting that epiphany changes the system gecko?

Comment on attachment 187314 [details] [diff] [review] moz-neterror protocol handler, rev. 1 +nsErrorPageProtocolHandler::NewChannel(nsIURI* aURI, nsIChannel* *aResult) +{ + nsCAutoString spec; + aURI->GetSpec(spec); this variable looks unused

Comment on attachment 187532 [details] [diff] [review] Use about:neterror unprivileged, rev. 1 Hrm... the #-mark checking is not really needed, right? You just added that for good measure, right? The about: protocol doesn't support nsIURL, so it sort of doesn't support reference fragments. Hrm... ok. r=darin

so what's the solution for embeddors?

Comment on attachment 187532 [details] [diff] [review] Use about:neterror unprivileged, rev. 1 the #-mark checking code was copied directly from nsAboutProtocolHandler, so that nsAboutRedirector and nsAboutProtocolHandler had the same parsing rules. biesi, nsAboutRedirector does not have any pref-overrides: I wouldn't mind a secondary patch to add such.

> the #-mark checking code was copied directly from nsAboutProtocolHandler how about moving the code in a common function? these two files end up in the same DSO...

This combines the hash-and-query search in one (very simple, surprisingly) utility function.

ah, actually, embeddors could probably just override the nsIAboutModule for about:neterror.

I'll try to get to this in the next day or two... I do think we want a separate bug (blocking 1.8) on having a simpler solution here for embeddors than overriding the about module.

Comment on attachment 188431 [details] [diff] [review] Use about:neterror unprivileged, rev. 2 Looks good. I wonder if it wouldn't make sense to give script execution privs to all about: URLs except about:blank. Maybe better safe than sorry, ok.

*** Bug 300003 has been marked as a duplicate of this bug. ***

Comment on attachment 188431 [details] [diff] [review] Use about:neterror unprivileged, rev. 2 sr=dveditz

Comment on attachment 188431 [details] [diff] [review] Use about:neterror unprivileged, rev. 2 first-review was from darin, switching products cleared the flag

I've done a lot of work for bug 280190. With this patch we aren't allowed to use any referenced css and image files? This would be very annoying, because the unstyled version doesn't look well. Inserting the css and image files into the netError.xhtml file will disallow themes to bring their own style. What's happen with the access to netError.dtd? Is this allowed? For the last patch v10 I changed the code to use a StringBundle. Was this work really useless? Benjamin, if you could give me (hopeful) answers, I would be glad.

> I've done a lot of work for bug 280190. With this patch we aren't allowed to use > any referenced css and image files? This would be very annoying, because the Why do you say that? > netError.xhtml file will disallow themes to bring their own style. What's I'm not sure I care all that much about theming this page. > changed the code to use a StringBundle. Was this work really useless? Yes, unfortunately.

(In reply to comment #34) > > netError.xhtml file will disallow themes to bring their own style. What's > > I'm not sure I care all that much about theming this page. I reverted my changes and it possible should work to include external CSS and images. So just one question. Benjamin, you moved all code from netError.js into netError.xhtml. What was your reason in attachment 188431 [details] [diff] [review]? Do you think we could still hold this file to swap out the processing script?

Fixed on trunk. I'm pretty sure that we can clear the security flag on this, because it's not on by default on the branches and there are no known trunk exploits.

The essential problem is not fixed yet. see bug 300830.