Attachment 157590 Details for Bug 93555 – Patch for landing

[patch] Patch for landing

bug-93555-20120809171518.patch (text/plain), 8.61 KB, created by Erik Arvidsson on 2012-08-09 17:15:45 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Erik Arvidsson

Created: 2012-08-09 17:15:45 PDT

Size: 8.61 KB

patch

obsolete

>Subversion Revision: 125191
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 19a03aee9e9cd0cda023fb95c0c01c63f524b0b6..6a0e5707a4d82f9cc01960a0d7489ee2f939b180 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,28 @@
>+2012-08-08  Erik Arvidsson  <arv@chromium.org>
>+
>+        [V8] Fix issues with [DoNotCheckSecurity] which depended on non ES5 compliant behavior
>+        https://bugs.webkit.org/show_bug.cgi?id=93555
>+
>+        Reviewed by Kentaro Hara.
>+
>+        Before this change we were relying on a V8 bug where instance properties could be shadow
>+        read only properties on the prototype. With this change we no longer depend on this incorrect
>+        V8 behavior and instead use a hidden v8 property that is only returned if the code can access
>+        the frame.
>+
>+        No new tests. No change in behavior.
>+
>+        * bindings/scripts/CodeGeneratorV8.pm:
>+        (GenerateDomainSafeFunctionGetter):
>+        (GenerateDomainSafeFunctionSetter):
>+        (GenerateNonStandardFunction):
>+        (GenerateImplementation):
>+        * bindings/scripts/test/V8/V8TestActiveDOMObject.cpp:
>+        (WebCore::TestActiveDOMObjectV8Internal::postMessageAttrGetter):
>+        (WebCore::TestActiveDOMObjectV8Internal::TestActiveDOMObjectDomainSafeFunctionSetter):
>+        (TestActiveDOMObjectV8Internal):
>+        (WebCore::ConfigureV8TestActiveDOMObjectTemplate):
>+
> 2012-08-09  Iain Merrick  <husky@chromium.org>
> 
>         [chromium] Enable kerning on Android
>diff --git a/Source/WebCore/bindings/scripts/CodeGeneratorV8.pm b/Source/WebCore/bindings/scripts/CodeGeneratorV8.pm
>index 9edc0af827ff05b1d55d479262f7150b7b961e27..4e8b44e2489386603bdceabab7c7b66a13534bd3 100644
>--- a/Source/WebCore/bindings/scripts/CodeGeneratorV8.pm
>+++ b/Source/WebCore/bindings/scripts/CodeGeneratorV8.pm
>@@ -773,12 +773,39 @@ static v8::Handle<v8::Value> ${funcName}AttrGetter(v8::Local<v8::String> name, c
>         static v8::Persistent<v8::FunctionTemplate> sharedTemplate = v8::Persistent<v8::FunctionTemplate>::New($newTemplateString);
>         return sharedTemplate->GetFunction();
>     }
>+
>+    v8::Local<v8::Value> hiddenValue = info.This()->GetHiddenValue(name);
>+    if (!hiddenValue.IsEmpty())
>+        return hiddenValue;
>+
>     return privateTemplate->GetFunction();
> }
> 
> END
> }
> 
>+sub GenerateDomainSafeFunctionSetter
>+{
>+    my $implClassName = shift;
>+    my $className = "V8" . $implClassName;
>+
>+    push(@implContentDecls, <<END);
>+static void ${implClassName}DomainSafeFunctionSetter(v8::Local<v8::String> name, v8::Local<v8::Value> value, const v8::AccessorInfo& info)
>+{
>+    INC_STATS("DOM.$implClassName._set");
>+    v8::Handle<v8::Object> holder = V8DOMWrapper::lookupDOMWrapper(${className}::GetTemplate(), info.This());
>+    if (holder.IsEmpty())
>+        return;
>+    ${implClassName}* imp = ${className}::toNative(holder);
>+    if (!BindingSecurity::shouldAllowAccessToFrame(BindingState::instance(), imp->frame()))
>+        return;
>+
>+    info.This()->SetHiddenValue(name, value);
>+}
>+
>+END
>+}
>+
> sub GenerateConstructorGetter
> {
>     my $dataNode = shift;
>@@ -2277,25 +2304,14 @@ sub GenerateNonStandardFunction
> 
>     if ($attrExt->{"DoNotCheckSecurity"} &&
>         ($dataNode->extendedAttributes->{"CheckSecurity"} || $interfaceName eq "DOMWindow")) {
>-        # Mark the accessor as ReadOnly and set it on the proto object so
>-        # it can be shadowed. This is really a hack to make it work.
>-        # There are several sceneria to call into the accessor:
>-        #   1) from the same domain: "window.open":
>-        #      the accessor finds the DOM wrapper in the proto chain;
>-        #   2) from the same domain: "window.__proto__.open":
>-        #      the accessor will NOT find a DOM wrapper in the prototype chain
>-        #   3) from another domain: "window.open":
>-        #      the access find the DOM wrapper in the prototype chain
>-        #   "window.__proto__.open" from another domain will fail when
>-        #   accessing '__proto__'
>-        #
>-        # The solution is very hacky and fragile, it really needs to be replaced
>-        # by a better solution.
>-        $property_attributes .= " | v8::ReadOnly";
>+        # Functions that are marked DoNotCheckSecurity are always readable but if they are changed
>+        # and then accessed on a different domain we do not return the underlying value but instead
>+        # return a new copy of the original function. This is achieved by storing the changed value
>+        # as hidden property.
>         push(@implContent, <<END);
> 
>     // $commentInfo
>-    ${conditional}$template->SetAccessor(v8::String::New("$name"), ${interfaceName}V8Internal::${name}AttrGetter, 0, v8Undefined(), v8::ALL_CAN_READ, static_cast<v8::PropertyAttribute>($property_attributes));
>+    ${conditional}$template->SetAccessor(v8::String::New("$name"), ${interfaceName}V8Internal::${name}AttrGetter, ${interfaceName}V8Internal::${interfaceName}DomainSafeFunctionSetter, v8Undefined(), v8::ALL_CAN_READ, static_cast<v8::PropertyAttribute>($property_attributes));
> END
>         return;
>     }
>@@ -2644,6 +2660,7 @@ END
>     my $namedPropertyGetter;
>     my @enabledPerContextFunctions;
>     my @normalFunctions;
>+    my $needsDomainSafeFunctionSetter = 0;
>     # Generate methods for functions.
>     foreach my $function (@{$dataNode->functions}) {
>         my $isCustom = $function->signature->extendedAttributes->{"Custom"} || $function->signature->extendedAttributes->{"V8Custom"};
>@@ -2668,6 +2685,7 @@ END
>         if (($dataNode->extendedAttributes->{"CheckSecurity"} || ($interfaceName eq "DOMWindow")) && $function->signature->extendedAttributes->{"DoNotCheckSecurity"}) {
>             if (!$isCustom || $function->{overloadIndex} == 1) {
>                 GenerateDomainSafeFunctionGetter($function, $implClassName);
>+                $needsDomainSafeFunctionSetter = 1;
>             }
>         }
> 
>@@ -2679,6 +2697,10 @@ END
>         }
>     }
> 
>+    if ($needsDomainSafeFunctionSetter) {
>+        GenerateDomainSafeFunctionSetter($implClassName);
>+    }
>+
>     # Attributes
>     my $attributes = $dataNode->attributes;
> 
>diff --git a/Source/WebCore/bindings/scripts/test/V8/V8TestActiveDOMObject.cpp b/Source/WebCore/bindings/scripts/test/V8/V8TestActiveDOMObject.cpp
>index 9ea8e0ce830841bfc211d592acb0cb08243e4a1e..5121a727e9092ce398c3234db5d372b9cddb262b 100644
>--- a/Source/WebCore/bindings/scripts/test/V8/V8TestActiveDOMObject.cpp
>+++ b/Source/WebCore/bindings/scripts/test/V8/V8TestActiveDOMObject.cpp
>@@ -87,9 +87,27 @@ static v8::Handle<v8::Value> postMessageAttrGetter(v8::Local<v8::String> name, c
>         static v8::Persistent<v8::FunctionTemplate> sharedTemplate = v8::Persistent<v8::FunctionTemplate>::New(v8::FunctionTemplate::New(TestActiveDOMObjectV8Internal::postMessageCallback, v8Undefined(), v8::Signature::New(V8TestActiveDOMObject::GetRawTemplate())));
>         return sharedTemplate->GetFunction();
>     }
>+
>+    v8::Local<v8::Value> hiddenValue = info.This()->GetHiddenValue(name);
>+    if (!hiddenValue.IsEmpty())
>+        return hiddenValue;
>+
>     return privateTemplate->GetFunction();
> }
> 
>+static void TestActiveDOMObjectDomainSafeFunctionSetter(v8::Local<v8::String> name, v8::Local<v8::Value> value, const v8::AccessorInfo& info)
>+{
>+    INC_STATS("DOM.TestActiveDOMObject._set");
>+    v8::Handle<v8::Object> holder = V8DOMWrapper::lookupDOMWrapper(V8TestActiveDOMObject::GetTemplate(), info.This());
>+    if (holder.IsEmpty())
>+        return;
>+    TestActiveDOMObject* imp = V8TestActiveDOMObject::toNative(holder);
>+    if (!BindingSecurity::shouldAllowAccessToFrame(BindingState::instance(), imp->frame(), ReportSecurityError))
>+        return;
>+
>+    info.This()->SetHiddenValue(name, value);
>+}
>+
> } // namespace TestActiveDOMObjectV8Internal
> 
> static const BatchedAttribute TestActiveDOMObjectAttrs[] = {
>@@ -119,7 +137,7 @@ static v8::Persistent<v8::FunctionTemplate> ConfigureV8TestActiveDOMObjectTempla
>     proto->Set(v8::String::New("excitingFunction"), v8::FunctionTemplate::New(TestActiveDOMObjectV8Internal::excitingFunctionCallback, v8Undefined(), excitingFunctionSignature));
> 
>     // Function 'postMessage' (ExtAttr: 'DoNotCheckSecurity')
>-    proto->SetAccessor(v8::String::New("postMessage"), TestActiveDOMObjectV8Internal::postMessageAttrGetter, 0, v8Undefined(), v8::ALL_CAN_READ, static_cast<v8::PropertyAttribute>(v8::DontDelete | v8::ReadOnly));
>+    proto->SetAccessor(v8::String::New("postMessage"), TestActiveDOMObjectV8Internal::postMessageAttrGetter, TestActiveDOMObjectV8Internal::TestActiveDOMObjectDomainSafeFunctionSetter, v8Undefined(), v8::ALL_CAN_READ, static_cast<v8::PropertyAttribute>(v8::DontDelete));
> 
>     // Custom toString template
>     desc->Set(getToStringName(), getToStringTemplate());

Actions: View | Formatted Diff | Diff

Attachments on bug 93555: 157341 | 157541 | 157590