Closed Bug 300830 Opened 19 years ago Closed 19 years ago

new XUL error page (about:neterror) can load privileged about: urls

Categories

(Core :: Security, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: sync2d, Assigned: dveditz)

Details

(Keywords: fixed1.8, Whiteboard: [sg:fix] [ready to land])

Attachments

(2 files)

User-Agent:       Mozilla/4.0 (compatible; MSIE 6.0; Windows NT 5.1)
Build Identifier: Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8b3) Gecko/20050714 Firefox/1.0+ (2005071411)

Since bug 292624 is fixed, XUL error pages no longer have chrome privileges.
However, a new XUL error page (about:neterror) can load other privileged
about: urls. XSS exploits can load XUL error pages first, and then
load about:config to abuse its privileges.

Reproducible: Always

Steps to Reproduce:
1. load XUL error page.
2. input following url into the location bar:
javascript: location = "about:config"; void 0;
3. press enter key or Go button.

Actual Results:  
about:config can be loaded.


Expected Results:  
Security Error: Content at about:neterror?... may not load or link to about:config.
Is this something that is worth worrying about? What codepath would first be
able to load an error page and then use XSS to load about:config and then use
XSS to do a real attack?

Where is/are the codepath(s) that protect against this for about:blank?
I think the reporter is imagining an attack like this:

1. Malicious web page causes an error page to be loaded.
2. XSS injects script into the error page (using any sufficient XSS hole).
3. Error page loads about:config (using this bug).
4. XSS injects script into about:config (using any sufficient XSS hole).
5. Script in about:config does something bad.

Since this bug turns many XSS holes in Firefox into full compromises, it should
be fixed.
Flags: blocking1.8b4?
Flags: blocking-aviary1.0.6?
Attached file exploit testcase
testcase to demonstrate the problem.
currently, about:neterror is only in the tinderbox builds.
(In reply to comment #3)
> testcase to demonstrate the problem.
> currently, about:neterror is only in the tinderbox builds.

This feature is part of the Deer Park Alpha 2 release.
Assignee: dveditz → benjamin
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: blocking1.8b4? → blocking1.8b4+
Whiteboard: [sg:fix]
I don't know anything about this problem, I'm not a good assignee for this bug.
How do we prevent (for example) about:blank from linking to about:config?

Jesse, I don't see why this should block 1.0.6 since error pages are not on by
default on the branch, and about:neterror doesn't exist.
Whiteboard: [sg:fix] → [sg:fix] need better assignee or more information
(In reply to comment #5)
> I don't know anything about this problem, I'm not a good assignee for this bug.
> How do we prevent (for example) about:blank from linking to about:config?

http://lxr.mozilla.org/seamonkey/source/caps/src/nsScriptSecurityManager.cpp#1161
http://lxr.mozilla.org/seamonkey/source/caps/src/nsScriptSecurityManager.cpp#1257
Non-privileged about: urls are distinguished from privileged ones.
We've got the original problem on the branch still, not this one (although
they're close to the same). On that one we decided that xul error pages weren't
supported on the branch so the fix wasn't needed.
Flags: blocking-aviary1.0.7? → blocking-aviary1.0.7-
Argh -- this is worse than the original bug 292624. Originally the error pages
themselves were not privileged, but you could jump from there to a privileged
context. Now it *is* privileged. If you've found an XSS bug you don't need to go
any further than the error page itself :-(

test: in the location bar enter

javascript:try{alert(Components.stack)}catch(e){alert(e)};void(0)
Assignee: benjamin → dveditz
Whiteboard: [sg:fix] need better assignee or more information → [sg:fix]
(In reply to comment #8)
> Argh -- this is worse than the original bug 292624. Originally the error pages
> themselves were not privileged, but you could jump from there to a privileged
> context. Now it *is* privileged. If you've found an XSS bug you don't need to go
> any further than the error page itself :-(
> test: in the location bar enter
> javascript:try{alert(Components.stack)}catch(e){alert(e)};void(0)

When bug 292624 was reported (2005-05-02), the error pages were not privileged.
When bug 300003 was reported (2005-07-07), the error pages were privileged.
Now, the error pages have lost their privileges again.

p=error pages are privileged, l=error page can load chrome, u=error page url

Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8b2) Gecko/20050501 Firefox/1.0+
p=0 l=1 u=chrome://global/content/netError.xhtml?...
Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8b2) Gecko/20050623 Firefox/1.0+ (2005062310)
p=0 l=1 u=chrome://global/content/netError.xhtml?...
Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8b2) Gecko/20050623 Firefox/1.0+ (2005062312)
p=1 l=1 u=chrome://global/content/netError.xhtml?...
Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8b4) Gecko/20050715 Firefox/1.0+
p=0 l=0 u=about:neterror?...

http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&
module=MozillaTinderboxAll&branch=HEAD&branchtype=match&
sortby=Date&hours=2&date=explicit&mindate=2005-06-23+10%3A00&
maxdate=2005-06-23+12%3A00&cvsroot=%2Fcvsroot
I think that nsChromeProtocolHandler.cpp  rev 1.07 gave
privileges to the former (chrome://netError.xhtml) error pages.
This bug should simply be a matter of making whatever check we do for preventing
about:blank->about:config also apply to about:neterror->about:config.
I notice plain "about:" is not in this list, and does allow loading privileged
about:config etc. should it be added to this list, too?
Attachment #190905 - Flags: superreview?(cbiesinger)
Attachment #190905 - Flags: review?(benjamin)
Comment on attachment 190905 [details] [diff] [review]
make about:neterror? safe

sorry... I'm not a super-reviewer... (but the patch does look right to me)
Attachment #190905 - Flags: superreview?(cbiesinger)
Attachment #190905 - Flags: superreview?(jst)
Whiteboard: [sg:fix] → [sg:fix] need reviews bsmedberg/jst
Comment on attachment 190905 [details] [diff] [review]
make about:neterror? safe

I'm actually worried about all these EqualsLiteral matches in general... I
believe that we should be using StringBeginsWith or something similar: couldn't
URIs of the form about:blank?foo=bar skip this check?
Whiteboard: [sg:fix] need reviews bsmedberg/jst → [sg:fix] need reviews bsmedberg/jst [ETA ??]
Comment on attachment 190905 [details] [diff] [review]
make about:neterror? safe

I'll take the sr on this -- bsmedberg, can you mark review (dveditz is about to
answer your question)?
Attachment #190905 - Flags: superreview?(jst) → superreview?(shaver)
(In reply to comment #13)
> I'm actually worried about all these EqualsLiteral matches in general... I
> believe that we should be using StringBeginsWith or something similar: couldn't
> URIs of the form about:blank?foo=bar skip this check?

Yes, they'd not be counted as "about safe", which means web content couldn't
open them just as they can't open "about:config". If they could be opened then
an attacker could go from there to about:config -- but being able to open them
means about:config could be loaded directly.  Upshot: the EqualsLiterals aren't
exactly right, but they aren't a security problem.
Comment on attachment 190905 [details] [diff] [review]
make about:neterror? safe

ok
Attachment #190905 - Flags: review?(benjamin) → review+
Whiteboard: [sg:fix] need reviews bsmedberg/jst [ETA ??] → [sg:fix] need sr=shaver[ETA ??]
Comment on attachment 190905 [details] [diff] [review]
make about:neterror? safe

sr=shaver (I thought I marked that in Cali!)
Attachment #190905 - Flags: superreview?(shaver) → superreview+
Comment on attachment 190905 [details] [diff] [review]
make about:neterror? safe

who can land this while dveditz is away?
Attachment #190905 - Flags: approval1.8b4+
Whiteboard: [sg:fix] need sr=shaver[ETA ??] → [sg:fix] [ready to land]
checked in, branch and trunk.
Status: NEW → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Keywords: fixed1.8
Flags: testcase+
Group: security
Flags: in-testsuite+ → in-testsuite?
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: