Bindings: perform security check before downcasting to LocalDOMWindow.

third_party/WebKit/Source/bindings/templates/methods.cpp.tmpl

Index: third_party/WebKit/Source/bindings/templates/methods.cpp.tmpl
diff --git a/third_party/WebKit/Source/bindings/templates/methods.cpp.tmpl b/third_party/WebKit/Source/bindings/templates/methods.cpp.tmpl
index 91b400d223096f28298c027e5bb419db78bb0a46..27a7dcd8193e2c1560279ead5aa63e43ebaa9775 100644
--- a/third_party/WebKit/Source/bindings/templates/methods.cpp.tmpl
+++ b/third_party/WebKit/Source/bindings/templates/methods.cpp.tmpl
@@ -29,15 +29,20 @@ static void {{method.name}}{{method.overload_index}}Method{{world_suffix}}(const
     return;
   }
   {% endif %}
-  {% if interface_name == 'Window' and not method.is_cross_origin %}
+  {% set local_dom_window_only = interface_name == 'Window' and not method.is_cross_origin %}
+  {% if local_dom_window_only %}
+  {% if not method.is_check_security_for_receiver %}

[haraken, 2017/02/07 15:43:13]

Ditto.

   // Same-origin methods are never exposed via the cross-origin interceptors.
   // Since same-origin access requires a LocalDOMWindow, it is safe to downcast
   // here.
   LocalDOMWindow* impl = toLocalDOMWindow({{v8_class}}::toImpl(info.Holder()));
   {% else %}
+  {{cpp_class}}* uncheckedImpl = {{v8_class}}::toImpl(info.Holder());

[haraken, 2017/02/07 15:43:13]

Instead, can we check isLocalDOMWindow()? If it returns false, we can return
immediately.

If that's possible, we don't need to introduce |uncheckedImpl| and can share
more code between local_dom_window_only and !local_dom_window_only.

[dcheng, 2017/02/08 01:30:25]

Unfortunately, it's not very easy to do that. If we check and return on line 66
to share code, then we won't throw a SecurityError.

If we return earlier than that, we have to adapt any special early return
handling as well as throwing the exception ourselves: some of the attribute
helpers call v8SetReturnValue when doing an early return, for instance. So
that's also duplicated code.

One last alternative is to have BindingSecurity check itself... but that's also
unusual. Normally, having a nullptr means we did not set up bookkeeping on the
v8 Object correctly... so it would be nice not to weaken the checks in
BindingSecurity.

Given this, I opted to just keep this code here =/

+  {% endif %}{# not method.is_check_security_for_receiver #}
+  {% else %}
   {{cpp_class}}* impl = {{v8_class}}::toImpl(info.Holder());
-  {% endif %}
-  {% endif %}
+  {% endif %}{# local_dom_window_only #}
+  {% endif %}{# not method.is_static #}
 
   {# Security checks #}
   {% if method.is_check_security_for_receiver %}
@@ -53,11 +58,18 @@ static void {{method.name}}{{method.overload_index}}Method{{world_suffix}}(const
     }
   }
   {% else %}{# interface_name == 'EventTarget' #}
+  {% if local_dom_window_only %}
+  if (!BindingSecurity::shouldAllowAccessTo(currentDOMWindow(info.GetIsolate()), uncheckedImpl, exceptionState)) {
+  {% else %}
   if (!BindingSecurity::shouldAllowAccessTo(currentDOMWindow(info.GetIsolate()), impl, exceptionState)) {
+  {% endif %}{# local_dom_window_only #}
     return;
   }
+  {% if local_dom_window_only %}
+  LocalDOMWindow* impl = toLocalDOMWindow(uncheckedImpl);
+  {% endif %}{# local_dom_window_only #}
   {% endif %}{# interface_name == 'EventTarget' #}
-  {% endif %}
+  {% endif %}{# method.is_check_security_for_receiver #}
   {% if method.is_check_security_for_return_value %}
   {{define_exception_state}}
   if (!BindingSecurity::shouldAllowAccessTo(currentDOMWindow(info.GetIsolate()), {{method.cpp_value}}, exceptionState)) {