406337 - .call({valueOf:function(){return null}) produces the global object

Status: RESOLVED DUPLICATE of bug 314874

(Core :: JavaScript Engine, defect)

Description

[John Resig]

If you perform a .call(null) the context is (understandably) set to be the global object. However, if you pass in an object whose valueOf() returns null, then the context is also set to be the window object.

In Firefox:
    function foo() { print(this); }

    foo()
    [object Window]

    foo.call({})
    [object Object]

    foo.call(null)
    [object Window]

    foo.call({valueOf: funct

In Webkit:
    function foo() { print(this); }

    foo()
    [object DOMWindow]

    foo.call({})
    [object Object]

    foo.call(null)
    [object DOMWindow]

    foo.call({valueOf: function(){return null;}})
    [object Object] 

I'm not sure if a spec change is required in order to make this change come about, but it seems like it may be a bug on our end.

Additionally, this bug is making it very hard for security projects, like Caja, to secure their code, since any object could be a possible attack vector.

Comment 1

[Jeff Walden [:Waldo]]

10.2.3
The caller provides the this value.  If the this value provided by the caller is not an object (including the case where it is null), then the this value is the global object.

This is a bug on our part, but I'm not sure it's as alarming as you make it out to be.  Does Caja really allow unmodified |this| accesses?  I would be astounded if it did.  Sadly they don't seem to have an equivalent to Facebook's test console, so I can't test:

http://developer.facebook.com/tools.php?fbml

Facebook, for example, can't be bitten by this because any reference to |this| is rewritten to not return the actual value when |this| is the global object (or any of the other bad things, not that I think that checking for more than global is just belt-and-suspenders).  |this| would be the global object, but who cares if any use would be rewritten?

We should fix this, but I don't see how it's going to adversely affect any JavaScript sandboxes currently in development or production.

Comment 2

[John Resig]

As I understand it, checks are put in place, in Caja, to make sure that any calls to .call() or .apply() are pre-checked to make sure that they aren't null or undefined. Firefox was the only browser that was capable of using .valueOf(function(){return null}) as a vector. So, yes, they could do an extra layer of checks to mitigate this (Mark Miller could probably chime in here) but it seems like that having one less "hole" to worry about is always best.

Comment 3

[Jeff Walden [:Waldo]]

This is surely the most distressingly obvious attack, but what do they do about this:

 function foo() { return this; }
 foo().alert("hi");

Either you're rewriting foo() to pass a different |this|, you're checking its return value, or you're rewriting |this| to something else -- I don't see any other way to do it.  The first two hardly perform well, and the last only forces you to pay when you actually use |this|, surely the fairest solution.

Comment 4

[Jeff Walden [:Waldo]]

...and the second one's no defense at all against |foo| containing |this.alert('hi')|.

Comment 5

[John Resig]

They already account for 'this' (as I understand it, they add an extra check to see what the value of 'this' is at the top of every function that uses it and if it's the global object they abort). However (again, as I understand it) they aren't able to check for 'this' inside implementation-specific functions, allowing some attack vectors. After a little bit of work I did this:
  var obj = {};
  obj.test = obj.valueOf;
  obj.valueOf = function(){ return null; };
  obj.test.call(obj).alert("uh oh");

The above, at no time, ever references this or explicitly calls a global variable (or the global object) and yet is still able to get access to the global object.

I've called Mark Miller into this discussion again as I think he might be able to better answer the questions relating to the implementation of Caja.

Comment 6

[Jeff Walden [:Waldo]]

This is what I get in the latest SpiderMonkey shell:

js> this.alert = function(v) { print(v); };
function (v) {
    print(v);
}
js> var obj = {};
js>   obj.test = obj.valueOf;
function valueOf() {
    [native code]
}
js>   obj.valueOf = function(){ return null; };
function () {
    return null;
}
js>   obj.test.call(obj).alert("uh oh");
typein:5: TypeError: obj.test.call(obj).alert is not a function
js>   obj.test.call(obj) === obj
true

...which is all correct as far as I can see.  I don't think this problem exists on trunk.

I tried the commented code on the sample page you gave in the thread in Firefox 2, and the first time (before my original comment here) it didn't work, probably due to an unfortunate miscopy -- sorry about that, this wouldn't have taken so long otherwise.  :-\  With this one, however, it works (only in 2, not in 3):

javascript:o={};o.test=o.valueOf;o.valueOf=function(){return null;};o.test.call(o).alert("uh-oh");

This is a Firefox 2-only problem as far as I can tell.

Comment 7

[John Resig]

Nope, you're right - it's already fixed in trunk (just did some testing myself). I assume that Mark would argue that this should be fixed in Firefox 2.0.0.x but I really don't see that happening, considering the nature of the fixing that we're pushing out in the minor releases.

Well, thanks for your help then; sorry for the hassle.

Comment 8

[Brendan Eich [:brendan]]

Is this bug not a dup of bug 314874 ?

/be

Comment 9

[John Resig]

(In reply to comment #8)
> Is this bug not a dup of bug 314874 ?

Yes! It does appear to be so. I did a search before hand but, apparently, was not able to find it. Thanks for spotting it.