595207 - E4X function:: namespace allows recognizing user despite clearing private data

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Wladimir Palant]

Attachment: Testcase

The issue has been published by Gareth Heyes in his blog: http://www.thespanner.co.uk/2010/09/10/function-is-the-new-window/

I am still marking this bug as security sensitive just in case, the privacy implications aren't exactly pointed out in his post. It seems that using "@mozilla.org/js/function" as namespace only works after function:: namespace was used at least once. This allows to "tag" a user for a browser session, he can be recognized as a returning visitor despite clearing all private data as long as the browser isn't closed. The attached testcase demonstrates this - this page will recognize whether it was opened for the first time.

Reproduced with Firefox 3.6.9 and Minefield build 20100910 on Windows 7 x64.

Comment 1

[Brendan Eich [:brendan]]

The bit of information here is not per site, it is JSRuntime-wide, so global among all browser contexts (windows/frames/tabs/etc.). So two sites couldn't track separately.

Patch in a few.

/be

Comment 2

[Brendan Eich [:brendan]]

Attachment: fix

Comment 3

[Brendan Eich [:brendan]]

Comment on attachment 474205 [details] [diff] [review]
fix

Wrong patch.

/be

Comment 4

[Brendan Eich [:brendan]]

Attachment: fix, take 2

It is hard to ban anti-URIs (the interal one starting with '@' is one instance) that are used internally, since jsxml.cpp constructs QNames using a constructor call. No secret argument channel to advise the ctor by which to whitelist the internal caller.

So the cat's out of the bag and I need to minimize the patch (and time spent on it), so I'm fixing this bug by recognizing "@mozilla.org/js/function" as the URI of the function:: namespace.

/be

Comment 5

[Igor Bukanov]

Comment on attachment 474219 [details] [diff] [review]
fix, take 2

>diff --git a/js/src/jsxml.cpp b/js/src/jsxml.cpp
>--- a/js/src/jsxml.cpp
>+++ b/js/src/jsxml.cpp
>@@ -2785,24 +2785,31 @@ ReportBadXMLName(JSContext *cx, const Va
> 
> static JSBool
> IsFunctionQName(JSContext *cx, JSObject *qn, jsid *funidp)
> {
>     JSAtom *atom;
>     JSString *uri;
> 
>     atom = cx->runtime->atomState.lazy.functionNamespaceURIAtom;
>+    if (!atom) {
>+        Value v;
>+        if (!js_GetFunctionNamespace(cx, &v))
>+            return false;
>+        atom = cx->runtime->atomState.lazy.functionNamespaceURIAtom;
>+    }
>+
>     uri = GetURI(qn);
>-    if (uri && atom &&
>+    if (uri &&
>         (uri == ATOM_TO_STRING(atom) ||
>          js_EqualStrings(uri, ATOM_TO_STRING(atom)))) {
>         return JS_ValueToId(cx, STRING_TO_JSVAL(GetLocalName(qn)), funidp);
>     }
>     *funidp = JSID_VOID;
>-    return JS_TRUE;
>+    return true;
> }

This looks reasonable but what about simply making functionNamespaceURIAtom non-lazy?

Comment 6

[Brendan Eich [:brendan]]

Let me see how that patch compares.

/be

Comment 7

[Igor Bukanov]

Attachment: being non-lazy

The patch makes functionNamespaceURIAtom non-lazy.

Comment 8

[Brendan Eich [:brendan]]

Comment on attachment 475032 [details] [diff] [review]
being non-lazy

Where's the test? Need to hg add it, maybe.

Ok, cleanups aside this is conceptually simpler, but is the minimal patch (take out all cleanups) actually smaller?

I hate taking on startup cost for E4X. r=me in trade.

Igor,could you please take this bug?

/be

Comment 9

[Brendan Eich [:brendan]]

I don't see why this bug should be s-s. The issue is already known, right? It's a one-bit, one-time-per-session-per-site tracking problem.

/be

Comment 10

[Brendan Eich [:brendan]]

Jesse, could use your refereeing here -- if you agree with comment 9, please open this bug.

/be

Comment 11

[Igor Bukanov]

Attachment: being non-lazy v2

Here is a minimized patch. Clearly it is not smaller than the attachment 4 [details] [diff] [review] but it also fixes a similar problem in JSObject::getCompartment.

As regarding the startup cost if we just remove the lazy atoms then simpler code with less if checks for null atoms in JS_ResolveStandardClass may well offset that. Besides, if the cost is measurable, then we may want to use something like static strings for atoms (bug 596229).

Comment 12

[Igor Bukanov]

http://hg.mozilla.org/tracemonkey/rev/50b1fa3d2de8

Comment 13

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/50b1fa3d2de8