When an http server redirects the user to a local file, XMLHttpRequest gets tricked into thinking the page came from the http server.

i can confirm this with win2k build 20020428..

I am marking this bug Security-Sensitive. That means only people in the Bugzilla security group and everyone whose email appears in the fields of the bug (reporter, etc.) will be able to view it. If anyone objects, simply uncheck the box above.

The folks at greymagic found this bug. Just so there's no confusion, I take no responsability for finding this bug.

BTW: http://www.theregister.co.uk/content/4/25079.html It looks like the problem is that http doesn't do any security handling itsself. Instead, it relies on docshell to call CheckLoadURI in its OnRedirect handler - see http://lxr.mozilla.org/seamonkey/source/uriloader/base/nsDocLoader.cpp#1270 darin, Mitch - should http be doing its own security checks? Is there ever a case where we don't want these run?

In 1.0RC1, this test case crashes for me on linux. A local test case sometimes crashes, and sometimes hangs, depending on the details of my redirect. I don't have a branch debug build to work our where this is happening. On the trunk, using that test page, I can load chrome urls, and have the text appear. I can't load js ones, though, because the channel doesn't provide an nsIInterfaceRequestor for us to get teh script global from, so I hit the assertion in nsJSThunk::EvaluateScript.

Any point in keeping this closed now that the Register has pasted the (trivial) sample code, as well as the GreyMagic advisory page having a working demonstration?

cls and timeless are going to turn off xmlextras in the default build until we have a fix, so I'm adding them to the cc:. Whatever fix-patch we end up with can tweak configure.in again.

Comment on attachment 81685 [details] [diff] [review] remove xmlextras from configure.in Noted on IRC but for the record, r=cls. FYI, this change is going to remove xmlextras from the build completely. To enable it in a build, you'll have to explicitly specify it: e.g. --enable-extensions=all,xmlextras

cc'ing heikki the module owner for xmlextras

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

This bug is also affected by document.load(). To test this put load.html and redir.asp on a webserver. point your browser to http://<yourwebserver>/load.html. load.html will display c:/test.xml in your browser.

I don't think there is point in keeping this secret, it is public knowledge now (articles at The Register etc.). This also affects document.load(), which you cannot disable by disabling xmlextras from the build. This needs a real fix. Marking nsbeta1+ and topembed+ (pretty sure Jud would not mind;).

two solutions: 1) make sure we have an nsIHttpEventSink::OnRedirect implementation whenever loading http URLs. 2) make http call CheckLoadURI itself. i'd prefer to go the route of solution 1 if at all possible. i know it's riskier, but it would keep things more modular (necko would have to depend on caps).

this patch ensures that CheckLoadURI is called for all HTTP redirects.

can people try out this patch and verify that it does indeed fix this bug? thx!

ok, i verified this patch against an XMLHttpRequest based exploit. if inside the netscape firewall, see http://unagi.mcom.com/~darinf/test.html if you have a local file on your system with the URL file:///tmp/test.txt, you'll see it's contents appear in an alert box when you load test.html. if you apply my patch, you'll no longer see any text in the alert box.

-> me

this patch comments out the redundant code in nsDocLoader.cpp

adding the to beta stopper list. Who should r=/sr= this?

undoing the keyword, whiteboard and priority changes i wiped away with my last change. sorry. readding nsbeta1+ and topembed+ because it looks like those were removed accidently as well.

I tried to make document.load() tests for Apache, but they don't seem to work. These are on an internal Netscape server (Netscape people should be able to log in normally if you want to tweak things). http://green.nscp.aoltw.net/heikki/141061/load-win.html (c:\temp\test.txt) http://green.nscp.aoltw.net/heikki/141061/load-unix.html (/tmp/test.txt) Anyone who could test the load tests from Giscard with Darin's patch?

Oops, sorry, I can confirm this fixes the document.load() bug on Windows as well.

To test the load() tests, place test.xml file on your local disc. c:\temp\test.xml for Windows and /tmp/test.xml for Unix. That test file could be something as simple as: <doc>Foobar</doc>

Comment on attachment 81725 [details] [diff] [review] patch - same thing, with equivalent code commented out of nsDocLoader.cpp Looks good. r=mstoltz

putting back on the beta stopper list.

I'd like to keep this bug security-confidential for another day or two. If anyone objects, please send mail to security-group@mozilla.org and we'll discuss it there - NOT here in the bug.

patch manager is really good at bad collisions...

On second thought, let's open up the bug. Transparency is good.

Has anyone got a debug branch build to see if this fixes the crash, too?

Comment on attachment 81725 [details] [diff] [review] patch - same thing, with equivalent code commented out of nsDocLoader.cpp sr=heikki

The crash was another issue, it has been fixed in XMLHttpRequest on the trunk & branch. Harishd just made a patch for document.load() case. Basically the crash occurred when those objects were used to load non-XML data, especially if they contained stylesheets. I am making a testcase to see if this fixes the case of redirecting stylesheets.

Comment on attachment 81725 [details] [diff] [review] patch - same thing, with equivalent code commented out of nsDocLoader.cpp a=asa (on behalf of drivers) for checkin to the 1.0 branch

Create test.css file with contents p {color:red;} and place it in c:\temp\test.css, then point your browser into this URL http://green.nscp.aoltw.net/heikki/141061/style-win.html Confirmed that this patch fixes that as well. This bug was reported on bugtraq.

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

fixed-on-trunk

adding adt1.0.0+. Please check this into the branch as soon as possible and add the fixed1.0.0 keyword.

Also fixed on 1.0 branch.

shouldn't this be marked FIXED now?

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

marking FIXED

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

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

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

Verified on 2002-05-01-trunk and 2002-05-01-branch on WinNT 3 testcases: 1. URL 2. barrowma.com/redirtest.html 3. test provided by Heikki work fine.

I was playing around with the exploit example page at http://sec.greymagic.com/adv/gm001-ns/ and i was wondering about what appears to be a similar problem. After updating to the very latest (fixed) Moz, the file://whatever hole seems to be fixed. However, if i type, say, http://slashdot.org/users.pl?op=edituser into the textbox and click "sniff", the content that their site grabs from me includes what is supposed to be private data -- my email address and so on. Could an approach similar to this bug be used to, say, grab someone's stock portfolio information by sniffing a URL at Yahoo Finance?

Mike, chances are the answer is "yes", from what I remember of this code... could you please file a separate bug on it?

Mike: yup, you raise a good point... however, short of breaking compatibility with a large number of websites, there is very little we can do to avoid that security risk. IMO, concerned users ought to disable javascript anyways when potentially visiting a malicious website.

I can fix the problem Mike mentioned in XMLHttpRequest itself (maybe also for document.load but I am not sure). I'll do that in bug 133170.

Darin, what sites? Who uses this? And this seems like a serious problem, one that will show up. I don't want to ship like that. Heikki, do we need to make sure that the bug you mentioned stays on the 1.0 radar?

blizzard: i was referring primarily to document.load ... seems like it'd be common place for websites to document.load an URL that results in a redirect to a site under a different domain. so, how can we block cross-site redirects on document.load? no other browser does, so we'd just end up making the product incompatible with who-knows-what top100 website.

To get a version which includes the fix, are the nightly builds stable enough for download by end-users? Or should they wait for RC2? Any estimate yet on a release date for RC2?

document.load(), and XMLHttpRequest.open() already check if they are allowed to access the URL that is passed in, and fail if the security check fails. The fact that you can go around this check by using a redirect seems like a bug to me, clear and simple. I created two testcases that show this allows a hacker access to Bugzilla using my account, and access to W3C member-only sections using my account. The exploits are possible if I have logged in to Bugzilla (cookies remember my login and are sent automatically) or W3C (basic auth login remembered and sent automatically) during this session. http://green.nscp.aoltw.net/heikki/133170/xmlhttpbugzilla.html http://green.nscp.aoltw.net/heikki/133170/xmlhttpw3cauth.html XMLHttpRequest is the more serious issue here because it can give you access to documents regardless of format. document.load(), I believe, will only be able to give the hacker access to XML documents. I have a fix for XMLHttpRequest altready, and I am debugging similar fix for document.load() in bug 133170. I would advice moving this discussion there.

> Any estimate yet on a release date for RC2? Any day now.

> Any estimate yet on a release date for RC2? When all dependencies for bug 138000 are fixed, except the nsIFIle one

for what it's worth, PPEmbed from 5/6/02 1.0 branch still has this problem. should i reopen? removing verified1.0.0 keyword. we need traction on this asap, it's a beta blocker for embedding clients.

never mind, it appears we jumped the gun. this bug appears fixed in 5/6/02 ppembed. someone is toking up without sharing again. ;)

adding back verified1.0.0 based on pinkerton's most recent comments. It sounds like new issues are being handled in 133170.

I was away for a few days; after reading the new comments, i'm not sure whether you still want me to file a new bug as requested in #49, or if someone else is taking care of this potential problem.

Mike, that is being covered by bug 133170.

Darin: Will this solve Bug 102262? I've been wanting that to go away for a long time, but haven't had time to focus on it.

Verified on 2002-05-08-6.2.3 on WinNT. 3 testcases: 1. URL ("This is blocked by the security manager) 2. barrowma.com/redirtest.html (an empty alert is shown) 3. test provided by Heikki (text is not red) work fine.