CustomGetterSetter accessors should only work on objects from the same global object

[Oliver Hunt]

In order to be safe from information leakage we should make the CustomGetterSetter functions require that the |this| object and the argument (in the case of setters) be from that same origin as the getter/setter function itself.

This is simply a guaranteed hardening against leakage, and as we haven't historically exposed this kind of information it can't be a regression.

[Chris Dumez]

I am not sure I understand.

I believe the following should work:
crossOriginWindow = open("other-origin-url");
locationGetter = Object.getOwnPropertyDescriptor(window, "location").get
locationGetter.call(crossOriginWindow)

It is OK to access location cross-origin as per:
https://html.spec.whatwg.org/multipage/browsers.html#security-window

Firefox also allows this.

Yet, the getter is from originA and is called on a window in originB.

[Oliver Hunt]

(In reply to comment #1)
> I am not sure I understand.
> 
> I believe the following should work:
> crossOriginWindow = open("other-origin-url");
> locationGetter = Object.getOwnPropertyDescriptor(window, "location").get
> locationGetter.call(crossOriginWindow)
> 
> It is OK to access location cross-origin as per:
> https://html.spec.whatwg.org/multipage/browsers.html#security-window
> 
> Firefox also allows this.
> 
> Yet, the getter is from originA and is called on a window in originB.

Here's the concern -- while i may be able to get the accessor for originB.location from originA.location, the accessor itself should have the originA.location accessor, otherwise you have an instant (exploitable) cross origin violation. I question the real world value of the cross origin accessor, hence just disallowing it is a super easy way to protect against at least some of the holes this would introduce.

[Chris Dumez]

Let's take an example where cross-origin access is not allowed since 'location' can be accessed cross-origin. Let's choose window.name:

I tried the following:
crossOriginWindow = open("other-origin-url");
nameGetter = Object.getOwnPropertyDescriptor(window, "name").get
nameGetter.call(crossOriginWindow)
-> Correctly logs a cross-origin error and return undefined.

It does not really matter from where the getter gets from as it acts only on the 'thisValue' that is being passed to it. The getter ends up calling the following bindings code:

EncodedJSValue jsDOMWindowName(ExecState* state, JSObject* slotBase, EncodedJSValue thisValue, PropertyName)
{
    UNUSED_PARAM(state);
    UNUSED_PARAM(slotBase);
    UNUSED_PARAM(thisValue);
    auto* castedThis = toJSDOMWindow(JSValue::decode(thisValue));
    if (UNLIKELY(!castedThis)) {
        return throwGetterTypeError(*state, "DOMWindow", "name");
    }
    if (!BindingSecurity::shouldAllowAccessToDOMWindow(state, castedThis->wrapped()))
        return JSValue::encode(jsUndefined());
    auto& impl = castedThis->wrapped();
    JSValue result = jsStringWithCache(state, impl.name());
    return JSValue::encode(result);
}

As you can see, it only acts on thisValue (Gavin is in the process of dropping the slotBase parameter as it is unused) and it does the cross-origin check.

For attributes such as location, all the following works because we do not do security checks for this property:
- crossOriginWindow.location
Works in Safari9, Chrome, Firefox

- Object.getOwnDescriptor(crossOriginWindow, "location")
New in WebKit ToT, works in Firefox and Chrome

- Object.getOwnPropertyDescriptor(window, "location").get.call(crossOriginWindow)
New in WebKit ToT, works in Firefox. Does not work in Chrome because Chrome returns 'value' descriptors for window properties instead of get/set ones (which is not correct as per the spec)

Given the spec, the behavior of Firefox and the fact that you can access cross-origin some of the window properties directly in Safari already (e.g. crossOriginWindow.location), I do not see any reason why getOwnPropertyDescriptor() or property getters would not work cross-origin for these properties.

[David Kilzer (:ddkilzer)]

<rdar://problem/32005817>