Allocation JS objects in the failed access check callback

[Mihai Parparita]

I'm looking to fix https://webkit.org/b/43504, such that accessing window.location.href (and other properties) across domains will throw a SECURITY_ERR exception, to conform with the HTML5 spec. The most obvious place to do this in the Chrome V8 bindings is inside the failed access check callback that's registered here:

http://trac.webkit.org/browser/trunk/WebCore/bindings/v8/V8DOMWindowShell.cpp#L267

However, when I try to add a call to V8Proxy::setDomException (source at http://trac.webkit.org/browser/trunk/WebCore/bindings/v8/V8Proxy.cpp#L679, that ends up calling v8::ThrowException), this assert in heap-inl.h ends up firing: http://code.google.com/p/v8/source/browse/trunk/src/heap-inl.h#54

This is because the invocation of the failed access check callback that happens in Top::ReportFailedAccessCheck (http://code.google.com/p/v8/source/browse/trunk/src/top.cc#546) has a "AssertNoAllocation no_gc" variable, with the comment "// The callers of this method are not expecting a GC."

I was wondering what the logic was behind that comment, and if you had any suggestions for alternatives? In places where Top::ReportFailedAccessCheck is called, it looks like allocations happen around there too (e.g. in the GetPropertyWithCallback call above http://code.google.com/p/v8/source/browse/trunk/src/objects.cc#273).

One option I looked into was turning off v8-level access checks and then writing custom getters that contained the access check and exception throwing. While that worked, that led to a significant increase in custom binding code, and seemed error-prone (since the security checks were not done automatically, so it'd be easy to miss adding one when creating a new custom getter).

I was hoping I could set a "should throw an exception" flag inside the failed access check, but I didn't see an obvious place where I could hook into to actually throw it and still be in the context of executing the same statement.

Thanks,

Mihai

[Mads Sig Ager]

Mihai,

I will look into this.  There are a couple of places in V8 where the ReportFailedAccessCheck method is called where the code cannot currently handle a GC. I'll see if I can make it safe for the callback to allocate.

Is the intension that all cross-frame accesses are supposed to generate DOM security exceptions?

Cheers,    -- Mads

--
v8-users mailing list
v8-u...@googlegroups.com
http://groups.google.com/group/v8-users

[Mihai Parparita]

On Thu, Aug 12, 2010 at 6:53 AM, Mads Sig Ager <ag...@chromium.org> wrote:
> I will look into this.  There are a couple of places in V8 where the
> ReportFailedAccessCheck method is called where the code cannot currently
> handle a GC. I'll see if I can make it safe for the callback to allocate.

Great, thanks!

> Is the intension that all cross-frame accesses are supposed to generate DOM
> security exceptions?

As best as I can tell from my reading of the HTML 5 spec, yes:

http://www.whatwg.org/specs/web-apps/current-work/multipage/dom.html#security-document
http://www.whatwg.org/specs/web-apps/current-work/multipage/browsers.html#security-window
http://www.whatwg.org/specs/web-apps/current-work/multipage/history.html#security-location

Mihai

[Mads Sig Ager]

From V8 revision 5257 it should be safe to allocate in the failed access check callback. We expect to push this version to Chromium on Monday. At that point you should be able to make the bindings change in WebKit.

Thanks,    -- Mads

Mihai

[Mihai Parparita]

Awesome, thanks!

Mihai