[SCM] WebKit Debian packaging branch, webkit-1.2, updated. upstream/1.1.90-6072-g9a69373
oliver at apple.com
oliver at apple.com
Thu Apr 8 00:23:00 UTC 2010
The following commit has been merged in the webkit-1.2 branch:
commit 16f51fd6ed23ffefdefeba63386598e653285a2c
Author: oliver at apple.com <oliver at apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date: Mon Dec 7 13:06:18 2009 +0000
Object.getOwnPropertyDescriptor() allows cross-frame access
https://bugs.webkit.org/show_bug.cgi?id=32119
Reviewed by Maciej Stachowiak.
Make all implementations of getOwnPropertyDescriptor that have
cross domain restrictions simply fail immediately on cross domain
access, rather than trying to mimic the behaviour of normal
property access.
Test: http/tests/security/cross-frame-access-getOwnPropertyDescriptor.html
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@51757 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 05f3e95..47acbcb 100644
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,19 @@
+2009-12-06 Oliver Hunt <oliver at apple.com>
+
+ Reviewed by Maciej Stachowiak.
+
+ Object.getOwnPropertyDescriptor() allows cross-frame access
+ https://bugs.webkit.org/show_bug.cgi?id=32119
+
+ Add cross domain tests for getOwnPropertyDescriptor
+
+ * http/tests/security/cross-frame-access-getOwnPropertyDescriptor.html: Added.
+ * http/tests/security/resources/cross-frame-access.js:
+ (canGetDescriptor.set get catch):
+ (canGetDescriptor):
+ * http/tests/security/resources/cross-frame-iframe-for-get-test.html:
+ * http/tests/security/xss-DENIED-defineProperty.html:
+
2009-12-06 Kent Tamura <tkent at chromium.org>
Reviewed by Darin Adler.
diff --git a/LayoutTests/http/tests/security/cross-frame-access-getOwnPropertyDescriptor.html b/LayoutTests/http/tests/security/cross-frame-access-getOwnPropertyDescriptor.html
new file mode 100644
index 0000000..16df594
--- /dev/null
+++ b/LayoutTests/http/tests/security/cross-frame-access-getOwnPropertyDescriptor.html
@@ -0,0 +1,292 @@
+<html>
+<head>
+ <script src="resources/cross-frame-access.js"></script>
+ <script>
+ var windowConstructorPropertiesNotAllowed = [
+ "Attr",
+ "Audio",
+ "CDATASection",
+ "CSSPrimitiveValue",
+ "CSSRule",
+ "CSSStyleDeclaration",
+ "CSSValue",
+ "CharacterData",
+ "Comment",
+ "DOMException",
+ "DOMImplementation",
+ "DOMParser",
+ "Document",
+ "DocumentFragment",
+ "DocumentType",
+ "Element",
+ "Entity",
+ "EntityReference",
+ "EvalError",
+ "Event",
+ "HTMLAnchorElement",
+ "HTMLAudioElement",
+ "HTMLAppletElement",
+ "HTMLAreaElement",
+ "HTMLBRElement",
+ "HTMLBaseElement",
+ "HTMLBaseFontElement",
+ "HTMLBodyElement",
+ "HTMLButtonElement",
+ "HTMLCanvasElement",
+ "HTMLDListElement",
+ "HTMLDirectoryElement",
+ "HTMLDivElement",
+ "HTMLDocument",
+ "HTMLElement",
+ "HTMLFieldSetElement",
+ "HTMLFontElement",
+ "HTMLFormElement",
+ "HTMLFrameElement",
+ "HTMLFrameSetElement",
+ "HTMLHRElement",
+ "HTMLHeadElement",
+ "HTMLHeadingElement",
+ "HTMLHtmlElement",
+ "HTMLIFrameElement",
+ "HTMLImageElement",
+ "HTMLInputElement",
+ "HTMLIsIndexElement",
+ "HTMLLIElement",
+ "HTMLLabelElement",
+ "HTMLLegendElement",
+ "HTMLLinkElement",
+ "HTMLMapElement",
+ "HTMLMarqueeElement",
+ "HTMLMediaElement",
+ "HTMLMenuElement",
+ "HTMLMetaElement",
+ "HTMLModElement",
+ "HTMLOListElement",
+ "HTMLOptGroupElement",
+ "HTMLOptionElement",
+ "HTMLParagraphElement",
+ "HTMLParamElement",
+ "HTMLPreElement",
+ "HTMLQuoteElement",
+ "HTMLScriptElement",
+ "HTMLSelectElement",
+ "HTMLSourceElement",
+ "HTMLStyleElement",
+ "HTMLTableCaptionElement",
+ "HTMLTableCellElement",
+ "HTMLTableColElement",
+ "HTMLTableElement",
+ "HTMLTableRowElement",
+ "HTMLTableSectionElement",
+ "HTMLTextAreaElement",
+ "HTMLTitleElement",
+ "HTMLUListElement",
+ "HTMLVideoElement",
+ "Image",
+ "MutationEvent",
+ "Node",
+ "NodeFilter",
+ "Notation",
+ "Option",
+ "ProcessingInstruction",
+ "Range",
+ "RangeError",
+ "RangeException",
+ "ReferenceError",
+ "SyntaxError",
+ "Text",
+ "TypeError",
+ "URIError",
+ "XMLDocument",
+ "XMLHttpRequest",
+ "XMLSerializer",
+ "XPathEvaluator",
+ "XPathResult",
+ "XSLTProcessor"
+ ];
+
+ var windowFunctionPropertiesNotAllowed = [
+ "addEventListener",
+ "alert",
+ "atob",
+ "btoa",
+ "captureEvents",
+ "clearInterval",
+ "clearTimeout",
+ "confirm",
+ "find",
+ "getComputedStyle",
+ "getMatchedCSSRules",
+ "getSelection",
+ "moveBy",
+ "moveTo",
+ "open",
+ "print",
+ "prompt",
+ "releaseEvents",
+ "removeEventListener",
+ "resizeBy",
+ "resizeTo",
+ "scroll",
+ "scrollBy",
+ "scrollTo",
+ "setInterval",
+ "setTimeout",
+ "showModalDialog",
+ "stop",
+ "blur",
+ "close",
+ "focus"
+ ];
+
+ var windowAttributesPropertiesNotAllowed = [
+ "clientInformation",
+ "console",
+ "crypto",
+ "defaultStatus",
+ "defaultstatus",
+ "devicePixelRatio",
+ "document",
+ "embeds",
+ "eval",
+ "event",
+ "frameElement",
+ "images",
+ "innerHeight",
+ "innerWidth",
+ "locationbar",
+ "menubar",
+ "name",
+ "navigator",
+ "offscreenBuffering",
+ "onabort",
+ "onbeforeunload",
+ "onblur",
+ "onchange",
+ "onclick",
+ "ondblclick",
+ "onerror",
+ "onfocus",
+ "onkeydown",
+ "onkeypress",
+ "onkeyup",
+ "onload",
+ "onmousedown",
+ "onmousemove",
+ "onmouseout",
+ "onmouseover",
+ "onmouseup",
+ "onmousewheel",
+ "onreset",
+ "onresize",
+ "onscroll",
+ "onsearch",
+ "onselect",
+ "onsubmit",
+ "onunload",
+ "outerHeight",
+ "outerWidth",
+ "pageXOffset",
+ "pageYOffset",
+ "personalbar",
+ "plugins",
+ "prototype",
+ "screen",
+ "screenLeft",
+ "screenTop",
+ "screenX",
+ "screenY",
+ "scrollX",
+ "scrollY",
+ "scrollbars",
+ "status",
+ "statusbar",
+ "toolbar",
+ "closed",
+ "frames",
+ "history",
+ "length",
+ "opener",
+ "parent",
+ "self",
+ "top",
+ "window"
+ ];
+
+ window.onload = function()
+ {
+ if (window.layoutTestController) {
+ layoutTestController.dumpAsText();
+ layoutTestController.waitUntilDone();
+ }
+
+ if (window.layoutTestController) {
+ setTimeout(pollForTest, 1);
+ } else {
+ log("To run the test, click the button below when the opened window finishes loading.");
+ var button = document.createElement("button");
+ button.appendChild(document.createTextNode("Run Test"));
+ button.onclick = runTest;
+ document.body.appendChild(button);
+ }
+ }
+
+ pollForTest = function()
+ {
+ if (!layoutTestController.globalFlag) {
+ setTimeout(pollForTest, 1);
+ return;
+ }
+ runTest();
+ layoutTestController.notifyDone();
+ }
+
+ runTest = function()
+ {
+ window.targetWindow = frames[0];
+
+ log("\n----- tests for getting of not allowed properties -----\n");
+
+ log("\n----- tests for getting of not allowed Constructors -----\n");
+ for (var i = 0; i < windowConstructorPropertiesNotAllowed.length; i++) {
+ var property = windowConstructorPropertiesNotAllowed[i];
+ shouldBeFalse("canGetDescriptor(targetWindow, '" + property + "')");
+ }
+
+ log("\n----- tests for getting of not allowed Functions -----\n");
+ for (var i = 0; i < windowFunctionPropertiesNotAllowed.length; i++) {
+ var property = windowFunctionPropertiesNotAllowed[i];
+ shouldBeFalse("canGetDescriptor(targetWindow, '" + property + "')");
+ }
+
+ log("\n----- tests for getting of not allowed Attributes -----\n");
+ for (var i = 0; i < windowAttributesPropertiesNotAllowed.length; i++) {
+ var property = windowAttributesPropertiesNotAllowed[i];
+ if (property == "document")
+ log("Firefox allows access to 'document' but throws an exception when you access its properties.");
+ shouldBeFalse("canGetDescriptor(targetWindow, '" + property + "')");
+ }
+ log("----- tests access to cross domain location object -----");
+ window.targetLocation = targetWindow.location;
+ var locationProperties = [
+ "assign", "replace", "reload", "protocol", "host", "hostname", "port", "pathname", "search", "hash", "toString", "valueOf", "customProperty"
+ ];
+ for (var i = 0; i < locationProperties.length; i++)
+ shouldBeFalse("canGetDescriptor(targetLocation, '" + locationProperties[i] + "')");
+
+ log("----- tests access to cross domain history object -----");
+ window.targetHistory = targetWindow.history;
+ var historyProperties = [
+ "length", "back", "forward", "go", "pushState", "replaceState", "customProperty"
+ ];
+ for (var i = 0; i < historyProperties.length; i++)
+ shouldBeFalse("canGetDescriptor(targetHistory, '" + historyProperties[i] + "')");
+ }
+ </script>
+</head>
+<body>
+<p>This test checks cross-frame access security of getOwnPropertyDescriptor (https://bugs.webkit.org/show_bug.cgi?id=32119).</p>
+<iframe src="http://localhost:8000/security/resources/cross-frame-iframe-for-get-test.html" style=""></iframe>
+<pre id="console"></pre>
+</body>
+</html>
diff --git a/LayoutTests/http/tests/security/resources/cross-frame-access.js b/LayoutTests/http/tests/security/resources/cross-frame-access.js
index 92f3fe7..87d4a49 100644
--- a/LayoutTests/http/tests/security/resources/cross-frame-access.js
+++ b/LayoutTests/http/tests/security/resources/cross-frame-access.js
@@ -58,6 +58,18 @@ function canGet(keyPath)
}
}
+function canGetDescriptor(target, property)
+{
+ try {
+ var desc = Object.getOwnPropertyDescriptor(target, property);
+ // To deal with an idiosyncrasy in how binding objects work in conjunction with
+ // slot and descriptor delegates we also allow descriptor with undefined value/getter/setter
+ return desc !== undefined && (desc.value !== undefined || desc.get !== undefined || desc.set !== undefined);
+ } catch(e) {
+ return false;
+ }
+}
+
window.marker = { };
function canSet(keyPath, valuePath)
diff --git a/LayoutTests/http/tests/security/resources/cross-frame-iframe-for-get-test.html b/LayoutTests/http/tests/security/resources/cross-frame-iframe-for-get-test.html
index 2206f4d..0b0aead 100644
--- a/LayoutTests/http/tests/security/resources/cross-frame-iframe-for-get-test.html
+++ b/LayoutTests/http/tests/security/resources/cross-frame-iframe-for-get-test.html
@@ -4,6 +4,8 @@
window.existingCustomProperty = 1;
window.__proto__.windowPrototypeCustomProperty = 1;
window.Object.prototype.objectPrototypeCustomProperty = 1;
+ window.location.customProperty = 1;
+ window.history.customProperty = 1;
window.onload = function()
{
diff --git a/LayoutTests/http/tests/security/xss-DENIED-defineProperty.html b/LayoutTests/http/tests/security/xss-DENIED-defineProperty.html
index a4fffcf..5874ea4 100644
--- a/LayoutTests/http/tests/security/xss-DENIED-defineProperty.html
+++ b/LayoutTests/http/tests/security/xss-DENIED-defineProperty.html
@@ -16,12 +16,12 @@ function finish()
else
document.getElementById("console").innerHTML += "FAIL: cross-site assignment of new property was allowed!<br/>";
- if (Object.getOwnPropertyDescriptor(location, "hash").value.length == 0)
+ if (location.hash.length == 0)
document.getElementById("console").innerHTML += "PASS: cross-site assignment of location.hash not allowed<br/>";
else
document.getElementById("console").innerHTML += "FAIL: cross-site assignment of location.hash was allowed!<br/>";
- if (Object.getOwnPropertyDescriptor(location, "hash").value.length == 0)
+ if (location.search.length == 0)
document.getElementById("console").innerHTML += "PASS: cross-site assignment of location.search not allowed<br/>";
else
document.getElementById("console").innerHTML += "FAIL: cross-site assignment of location.search was allowed!<br/>";
diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
index 9b34358..1de35d6 100644
--- a/WebCore/ChangeLog
+++ b/WebCore/ChangeLog
@@ -1,3 +1,25 @@
+2009-12-06 Oliver Hunt <oliver at apple.com>
+
+ Reviewed by Maciej Stachowiak.
+
+ Object.getOwnPropertyDescriptor() allows cross-frame access
+ https://bugs.webkit.org/show_bug.cgi?id=32119
+
+ Make all implementations of getOwnPropertyDescriptor that have
+ cross domain restrictions simply fail immediately on cross domain
+ access, rather than trying to mimic the behaviour of normal
+ property access.
+
+ Test: http/tests/security/cross-frame-access-getOwnPropertyDescriptor.html
+
+ * bindings/js/JSDOMWindowCustom.cpp:
+ (WebCore::JSDOMWindow::getOwnPropertyDescriptor):
+ * bindings/js/JSHistoryCustom.cpp:
+ (WebCore::JSHistory::getOwnPropertyDescriptorDelegate):
+ * bindings/js/JSLocationCustom.cpp:
+ (WebCore::JSLocation::getOwnPropertyDescriptorDelegate):
+ * bindings/scripts/CodeGeneratorJS.pm:
+
2009-12-07 Steve Block <steveblock at google.com>
Reviewed by Adam Barth.
diff --git a/WebCore/bindings/js/JSDOMWindowCustom.cpp b/WebCore/bindings/js/JSDOMWindowCustom.cpp
index 8ba7b91..7139e5b 100644
--- a/WebCore/bindings/js/JSDOMWindowCustom.cpp
+++ b/WebCore/bindings/js/JSDOMWindowCustom.cpp
@@ -297,10 +297,10 @@ bool JSDOMWindow::getOwnPropertySlot(ExecState* exec, const Identifier& property
bool JSDOMWindow::getOwnPropertyDescriptor(ExecState* exec, const Identifier& propertyName, PropertyDescriptor& descriptor)
{
- // When accessing a Window cross-domain, functions are always the native built-in ones, and they
- // are not affected by properties changed on the Window or anything in its prototype chain.
- // This is consistent with the behavior of Firefox.
-
+ // Never allow cross-domain getOwnPropertyDescriptor
+ if (!allowsAccessFrom(exec))
+ return false;
+
const HashEntry* entry;
// We don't want any properties other than "close" and "closed" on a closed window.
@@ -323,11 +323,6 @@ bool JSDOMWindow::getOwnPropertyDescriptor(ExecState* exec, const Identifier& pr
return true;
}
- String errorMessage;
- bool allowsAccess = allowsAccessFrom(exec, errorMessage);
- if (allowsAccess && JSGlobalObject::getOwnPropertyDescriptor(exec, propertyName, descriptor))
- return true;
-
// We need this code here because otherwise JSDOMWindowBase will stop the search before we even get to the
// prototype due to the blanket same origin (allowsAccessFrom) check at the end of getOwnPropertySlot.
// Also, it's important to get the implementation straight out of the DOMWindow prototype regardless of
@@ -335,51 +330,13 @@ bool JSDOMWindow::getOwnPropertyDescriptor(ExecState* exec, const Identifier& pr
entry = JSDOMWindowPrototype::s_info.propHashTable(exec)->entry(exec, propertyName);
if (entry) {
if (entry->attributes() & Function) {
- if (entry->function() == jsDOMWindowPrototypeFunctionBlur) {
- if (!allowsAccess) {
- PropertySlot slot;
- slot.setCustom(this, nonCachingStaticFunctionGetter<jsDOMWindowPrototypeFunctionBlur, 0>);
- descriptor.setDescriptor(slot.getValue(exec, propertyName), ReadOnly | DontDelete | DontEnum);
- return true;
- }
- } else if (entry->function() == jsDOMWindowPrototypeFunctionClose) {
- if (!allowsAccess) {
- PropertySlot slot;
- slot.setCustom(this, nonCachingStaticFunctionGetter<jsDOMWindowPrototypeFunctionClose, 0>);
- descriptor.setDescriptor(slot.getValue(exec, propertyName), ReadOnly | DontDelete | DontEnum);
- return true;
- }
- } else if (entry->function() == jsDOMWindowPrototypeFunctionFocus) {
- if (!allowsAccess) {
- PropertySlot slot;
- slot.setCustom(this, nonCachingStaticFunctionGetter<jsDOMWindowPrototypeFunctionFocus, 0>);
- descriptor.setDescriptor(slot.getValue(exec, propertyName), ReadOnly | DontDelete | DontEnum);
- return true;
- }
- } else if (entry->function() == jsDOMWindowPrototypeFunctionPostMessage) {
- if (!allowsAccess) {
- PropertySlot slot;
- slot.setCustom(this, nonCachingStaticFunctionGetter<jsDOMWindowPrototypeFunctionPostMessage, 2>);
- descriptor.setDescriptor(slot.getValue(exec, propertyName), ReadOnly | DontDelete | DontEnum);
- return true;
- }
- } else if (entry->function() == jsDOMWindowPrototypeFunctionShowModalDialog) {
+ if (entry->function() == jsDOMWindowPrototypeFunctionShowModalDialog) {
if (!DOMWindow::canShowModalDialog(impl()->frame())) {
descriptor.setUndefined();
return true;
}
}
}
- } else {
- // Allow access to toString() cross-domain, but always Object.prototype.toString.
- if (propertyName == exec->propertyNames().toString) {
- if (!allowsAccess) {
- PropertySlot slot;
- slot.setCustom(this, objectToStringFunctionGetter);
- descriptor.setDescriptor(slot.getValue(exec, propertyName), ReadOnly | DontDelete | DontEnum);
- return true;
- }
- }
}
entry = JSDOMWindow::s_info.propHashTable(exec)->entry(exec, propertyName);
@@ -406,13 +363,8 @@ bool JSDOMWindow::getOwnPropertyDescriptor(ExecState* exec, const Identifier& pr
// precedence over the index and name getters.
JSValue proto = prototype();
if (proto.isObject()) {
- if (asObject(proto)->getPropertyDescriptor(exec, propertyName, descriptor)) {
- if (!allowsAccess) {
- printErrorMessage(errorMessage);
- descriptor.setUndefined();
- }
+ if (asObject(proto)->getPropertyDescriptor(exec, propertyName, descriptor))
return true;
- }
}
bool ok;
diff --git a/WebCore/bindings/js/JSHistoryCustom.cpp b/WebCore/bindings/js/JSHistoryCustom.cpp
index db2ebde..3076503 100644
--- a/WebCore/bindings/js/JSHistoryCustom.cpp
+++ b/WebCore/bindings/js/JSHistoryCustom.cpp
@@ -95,15 +95,15 @@ bool JSHistory::getOwnPropertySlotDelegate(ExecState* exec, const Identifier& pr
bool JSHistory::getOwnPropertyDescriptorDelegate(ExecState* exec, const Identifier& propertyName, PropertyDescriptor& descriptor)
{
- // When accessing History cross-domain, functions are always the native built-in ones.
- // See JSDOMWindow::getOwnPropertySlotDelegate for additional details.
-
- // Our custom code is only needed to implement the Window cross-domain scheme, so if access is
- // allowed, return false so the normal lookup will take place.
- String message;
- if (allowsAccessFromFrame(exec, impl()->frame(), message))
- return false;
-
+ if (!impl()->frame()) {
+ descriptor.setUndefined();
+ return true;
+ }
+
+ // Throw out all cross domain access
+ if (!allowsAccessFromFrame(exec, impl()->frame()))
+ return true;
+
// Check for the few functions that we allow, even when called cross-domain.
const HashEntry* entry = JSHistoryPrototype::s_info.propHashTable(exec)->entry(exec, propertyName);
if (entry) {
@@ -133,8 +133,7 @@ bool JSHistory::getOwnPropertyDescriptorDelegate(ExecState* exec, const Identifi
return true;
}
}
-
- printErrorMessageForFrame(impl()->frame(), message);
+
descriptor.setUndefined();
return true;
}
diff --git a/WebCore/bindings/js/JSLocationCustom.cpp b/WebCore/bindings/js/JSLocationCustom.cpp
index d765261..6c8e032 100644
--- a/WebCore/bindings/js/JSLocationCustom.cpp
+++ b/WebCore/bindings/js/JSLocationCustom.cpp
@@ -102,14 +102,9 @@ bool JSLocation::getOwnPropertyDescriptorDelegate(ExecState* exec, const Identif
return true;
}
- // When accessing Location cross-domain, functions are always the native built-in ones.
- // See JSDOMWindow::getOwnPropertySlotDelegate for additional details.
-
- // Our custom code is only needed to implement the Window cross-domain scheme, so if access is
- // allowed, return false so the normal lookup will take place.
- String message;
- if (allowsAccessFromFrame(exec, frame, message))
- return false;
+ // throw out all cross domain access
+ if (!allowsAccessFromFrame(exec, frame))
+ return true;
// Check for the few functions that we allow, even when called cross-domain.
const HashEntry* entry = JSLocationPrototype::s_info.propHashTable(exec)->entry(exec, propertyName);
@@ -133,8 +128,7 @@ bool JSLocation::getOwnPropertyDescriptorDelegate(ExecState* exec, const Identif
// FIXME: Other implementers of the Window cross-domain scheme (Window, History) allow toString,
// but for now we have decided not to, partly because it seems silly to return "[Object Location]" in
// such cases when normally the string form of Location would be the URL.
-
- printErrorMessageForFrame(frame, message);
+
descriptor.setUndefined();
return true;
}
diff --git a/WebCore/bindings/scripts/CodeGeneratorJS.pm b/WebCore/bindings/scripts/CodeGeneratorJS.pm
index 5c390dc..ebb6648 100644
--- a/WebCore/bindings/scripts/CodeGeneratorJS.pm
+++ b/WebCore/bindings/scripts/CodeGeneratorJS.pm
@@ -375,6 +375,14 @@ sub GenerateGetOwnPropertyDescriptorBody
my $namespaceMaybe = ($inlined ? "JSC::" : "");
my @getOwnPropertyDescriptorImpl = ();
+ if ($dataNode->extendedAttributes->{"CheckDomainSecurity"}) {
+ if ($interfaceName eq "DOMWindow") {
+ push(@implContent, " if (!static_cast<$className*>(thisObject)->allowsAccessFrom(exec))\n");
+ } else {
+ push(@implContent, " if (!allowsAccessFromFrame(exec, static_cast<$className*>(thisObject)->impl()->frame()))\n");
+ }
+ push(@implContent, " return false;\n");
+ }
if ($interfaceName eq "NamedNodeMap" or $interfaceName eq "HTMLCollection" or $interfaceName eq "HTMLAllCollection") {
push(@getOwnPropertyDescriptorImpl, " ${namespaceMaybe}JSValue proto = prototype();\n");
--
WebKit Debian packaging
More information about the Pkg-webkit-commits
mailing list