Changeset 51757 in webkit

Timestamp:

Dec 7, 2009 5:06:18 AM (14 years ago)

Author:

oliver@apple.com

Message:

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

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/http/tests/security/cross-frame-access-getOwnPropertyDescriptor.html (added)
• LayoutTests/http/tests/security/resources/cross-frame-access.js (modified) (1 diff)
• LayoutTests/http/tests/security/resources/cross-frame-iframe-for-get-test.html (modified) (1 diff)
• LayoutTests/http/tests/security/xss-DENIED-defineProperty.html (modified) (1 diff)
• WebCore/ChangeLog (modified) (1 diff)
• WebCore/bindings/js/JSDOMWindowCustom.cpp (modified) (4 diffs)
• WebCore/bindings/js/JSHistoryCustom.cpp (modified) (2 diffs)
• WebCore/bindings/js/JSLocationCustom.cpp (modified) (2 diffs)
• WebCore/bindings/scripts/CodeGeneratorJS.pm (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2009-12-06  Oliver Hunt  <oliver@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@chromium.org>
 

trunk/LayoutTests/http/tests/security/resources/cross-frame-access.js

@@ -54 +54 @@
     try {
         return eval("window." + keyPath) !== undefined;
+    } catch(e) {
+        return false;
+    }
+}
+
+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;

trunk/LayoutTests/http/tests/security/resources/cross-frame-iframe-for-get-test.html

@@ -5 +5 @@
         window.__proto__.windowPrototypeCustomProperty = 1; 
         window.Object.prototype.objectPrototypeCustomProperty = 1;
+        window.location.customProperty = 1;
+        window.history.customProperty = 1;
 
         window.onload = function()

trunk/LayoutTests/http/tests/security/xss-DENIED-defineProperty.html

@@ -17 +17 @@
         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

trunk/WebCore/ChangeLog

@@ +1 @@
+2009-12-06  Oliver Hunt  <oliver@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@google.com>
 

trunk/WebCore/bindings/js/JSDOMWindowCustom.cpp

@@ -298 +298 @@
 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;
     
@@ -324 +324 @@
     }
 
-    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.
@@ -336 +331 @@
     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;
             }
         }
@@ -407 +364 @@
     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;
-        }
     }
 

trunk/WebCore/bindings/js/JSHistoryCustom.cpp

@@ -96 +96 @@
 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);
@@ -134 +134 @@
         }
     }
-    
-    printErrorMessageForFrame(impl()->frame(), message);
+
     descriptor.setUndefined();
     return true;

trunk/WebCore/bindings/js/JSLocationCustom.cpp

@@ -103 +103 @@
     }
     
-    // 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.
@@ -134 +129 @@
     // 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;

trunk/WebCore/bindings/scripts/CodeGeneratorJS.pm

@@ -376 +376 @@
     
     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") {