- Revision
- 125232
- Author
- [email protected]
- Date
- 2012-08-09 18:52:13 -0700 (Thu, 09 Aug 2012)
Log Message
[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):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (125231 => 125232)
--- trunk/Source/WebCore/ChangeLog 2012-08-10 01:32:05 UTC (rev 125231)
+++ trunk/Source/WebCore/ChangeLog 2012-08-10 01:52:13 UTC (rev 125232)
@@ -1,3 +1,28 @@
+2012-08-09 Erik Arvidsson <[email protected]>
+
+ [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 David Grogan <[email protected]>
IndexedDB: intversion-long-queue.html fails an assert
Modified: trunk/Source/WebCore/bindings/scripts/CodeGeneratorV8.pm (125231 => 125232)
--- trunk/Source/WebCore/bindings/scripts/CodeGeneratorV8.pm 2012-08-10 01:32:05 UTC (rev 125231)
+++ trunk/Source/WebCore/bindings/scripts/CodeGeneratorV8.pm 2012-08-10 01:52:13 UTC (rev 125232)
@@ -773,12 +773,39 @@
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 @@
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 @@
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 @@
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 @@
}
}
+ if ($needsDomainSafeFunctionSetter) {
+ GenerateDomainSafeFunctionSetter($implClassName);
+ }
+
# Attributes
my $attributes = $dataNode->attributes;
Modified: trunk/Source/WebCore/bindings/scripts/test/V8/V8TestActiveDOMObject.cpp (125231 => 125232)
--- trunk/Source/WebCore/bindings/scripts/test/V8/V8TestActiveDOMObject.cpp 2012-08-10 01:32:05 UTC (rev 125231)
+++ trunk/Source/WebCore/bindings/scripts/test/V8/V8TestActiveDOMObject.cpp 2012-08-10 01:52:13 UTC (rev 125232)
@@ -87,9 +87,27 @@
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 @@
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());
