Jesse Ruderman Reporter
	Description • 18 years ago

http://jeremiahgrossman.blogspot.com/2006/12/i-know-if-youre-logged-in-anywhere.html describes how a malicious site can use onerror and <script src="..."> to determine whether you're logged into another site.  Loading an HTML page using <script src="..."> usually triggers a syntax error, but the exact error and line number depend on the HTML.

Firefox could prevent this kind of attack on most sites by giving less information to the onerror handler when a script loaded from another site has a syntax error.  I doubt this would break any sites other than ones trying to determine whether you're logged into another site.

I imagine that this could be done either by throwing same-origin exceptions when the site tries to access certain fields of the object passed to the onerror handler.  But it would probably be simpler to give the onerror handler an object that contains fewer details about the error.  The full syntax error could still be shown in the Error Console.

	Biju
	Comment 1 • 18 years ago

another one which can do almost the same.
bug 147777

	Jonas Sicking (:sicking) No longer reading bugmail consistently
	Comment 2 • 18 years ago

jst will have a go at this

	Blake Kaplan (:mrbkap) (inactive)
	Comment 4 • 17 years ago

Comment on attachment 271141 [details] [diff] [review]
Don't give error handlers error info for xorigin errors.

I asked Johnny if the filename could potentially leak information as well (due to redirects if logged in) and it looks like we'll compile the script with whatever URI was passed in the src= attribute as the filename.

	Brendan Eich [:brendan]
	Comment 5 • 17 years ago

Comment on attachment 271141 [details] [diff] [review]
Don't give error handlers error info for xorigin errors.

>@@ -333,8 +335,34 @@ NS_ScriptErrorReporter(JSContext *cx,
>             nsScriptErrorEvent errorevent(PR_TRUE, NS_LOAD_ERROR);
> 
>             errorevent.fileName = fileName.get();
>-            errorevent.errorMsg = msg.get();
>-            errorevent.lineNr = report ? report->lineno : 0;
>+
>+            nsCOMPtr<nsIScriptObjectPrincipal> sop(do_QueryInterface(win));
>+            nsIPrincipal *p = sop->GetPrincipal();
>+
>+            PRBool sameOrigin = PR_FALSE;
>+
>+            if (p) {
>+              nsCOMPtr<nsIURI> errorURI;
>+              NS_NewURI(getter_AddRefs(errorURI),
>+                        NS_ConvertUTF16toUTF8(fileName).get());

Couldn't you use report->filename directly? It's either ASCII or (if JS_C_STRINGS_ARE_UTF8) UTF-8 already.

>+
>+              nsCOMPtr<nsIURI> codebase;
>+              p->GetURI(getter_AddRefs(codebase));
>+
>+              if (errorURI && codebase) {
>+                sameOrigin =
>+                  NS_SUCCEEDED(sSecurityManager->
>+                               CheckSameOriginURI(errorURI, codebase));

Ideally we'd have a followup bug to provide error principal, which ought to be the subject principal, to the error reporter -- then you could use CheckSameOriginPrincipal here. But it seems you can't just get the subject principal here, because it may be too late (we may be reporting an uncaught exception and the script that failed to catch has unwound).

If this seems sane, please file that followup and cite it here with a FIXME: comment.

sr=me with the above taken into account.

/be

	Johnny Stenback (:jst) Assignee
	Comment 6 • 17 years ago

This deals with Brendan's comments and also fixes a problem with not null checking report like the surrounding code does (bug 387478 filed to clean that up).

	Johnny Stenback (:jst) Assignee
	Comment 7 • 17 years ago

I filed followup bug 387476 and bug 387477, marking bug FIXED.

	İsmail Dönmez
	Comment 9 • 17 years ago

Any chance of a Firefox 2.0.0.x backport ?

	Boris Zbarsky [:bzbarsky]
	Comment 10 • 17 years ago

Hmm.  This patch makes me pretty unhappy, with the "URI == principal" thing.  Is there any way to store the principal of the script that throws in exceptions?

	Boris Zbarsky [:bzbarsky]
	Comment 11 • 16 years ago

Note that this did cause trouble for at least one production site (or at least for the debugging thereof).  See bug 467439.

	Daniel Veditz [:dveditz]
	Comment 12 • 16 years ago

Version for 1.8 branch. The only minor change (besides context) is getting the nsIScriptObjectPrincipal from globalObject directly instead of from "win", a nsPIDOMWindow QI'd from globalObject that isn't used on the 1.8 branch.

	Daniel Veditz [:dveditz]
	Comment 13 • 16 years ago

Comment on attachment 351093 [details] [diff] [review]
1.8-branch version

Checked into 1.8 branch

	Daniel Veditz [:dveditz]
	Comment 14 • 16 years ago

I guess technically we've branched so this is fixed for the next one, and I have to check into the relbranch to fix it for 1.8.1.19

	Daniel Veditz [:dveditz]
	Comment 15 • 16 years ago

Checked into 1.8.1.19 relbranch after verifying the fix in a tinderbox run (mac).

	Al Billings [:abillings - ex-MoCo]
	Comment 16 • 16 years ago

Using the proof of concept at http://ha.ckers.org/weird/javascript-website-login-checker.html, I'm still seeing a difference in the error console log if I'm logged into a site and if I'm not.

For Google, if I'm logged in and run the check in the PoC, I get the following in the error console:

Error: syntax error
Source File: https://mail.google.com/mail/
Line: 1
Source Code:
<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN"

If I'm *not* logged in, I get:

Error: invalid XML attribute value
Source File: https://www.google.com/accounts/ServiceLogin?service=mail&passive=true&rm=false&continue=http%3A%2F%2Fmail.google.com%2Fmail%2F%3Fui%3Dhtml%26zy%3Dl&bsv=1k96igf4806cy&ltmpl=default&ltmplcache=2
Line: 5, Column: 12
Source Code:
<style type=text/css>

Unless I'm misunderstanding things, we should not be giving this information.

	Boris Zbarsky [:bzbarsky]
	Comment 17 • 16 years ago

The error console reports the full error.  The scripted onerror handler in the webpage is what gets no information cross-site.

	Al Billings [:abillings - ex-MoCo]
	Comment 18 • 16 years ago

Ah! I've verified it with the PoC then using Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.1.19) Gecko/2008120316 Firefox/2.0.0.19. It is fixed.

	Alexander Sack
	Comment 19 • 16 years ago

Comment on attachment 351093 [details] [diff] [review]
1.8-branch version

a=asac for 1.8.0 branch

	Nick Thomas [:nthomas] (UTC+12)
	Comment 21 • 16 years ago

(In reply to comment #15)
> Checked into 1.8.1.19 relbranch after verifying the fix in a tinderbox run
> (mac).

The win32 builds that shipped today for Firefox 2.0.0.19 do not contain this fix. We accidentally shipped the first build we did, rather than the respin which took this fix. Not sure what to do with the keywords to capture this.

	Alexander Sack
	Comment 22 • 16 years ago

Nick, did you ship the FIREFOX_2_0_0_19_BUILD1 tag or has the FIREFOX_2_0_0_19_RELEASE tag not properly pushed forward?

	bhearsum@mozilla.com (:bhearsum)
	Comment 23 • 16 years ago

(In reply to comment #22)
> Nick, did you ship the FIREFOX_2_0_0_19_BUILD1 tag or has the
> FIREFOX_2_0_0_19_RELEASE tag not properly pushed forward?

The FIREFOX_2_0_0_19_RELEASE tag was moved forward to match FIREFOX_2_0_0_19_BUILD2