Title: [95489] trunk
Revision
95489
Author
aba...@webkit.org
Date
2011-09-19 15:57:30 -0700 (Mon, 19 Sep 2011)

Log Message

[V8] document.all gets confused about its prototype chain
https://bugs.webkit.org/show_bug.cgi?id=68393

Reviewed by Eric Seidel.

Source/WebCore: 

GetRealNamedPropertyInPrototypeChain doesn't call interceptors, so it's
not a good idea to use its return value.  It turns out that all the
callers of the API only cared about whether it returns a null handle.

Test: http/tests/security/document-all.html

* bindings/v8/V8Collection.h:
(WebCore::collectionNamedPropertyGetter):
* bindings/v8/custom/V8DOMStringMapCustom.cpp:
(WebCore::V8DOMStringMap::namedPropertyDeleter):
(WebCore::V8DOMStringMap::namedPropertySetter):
* bindings/v8/custom/V8HTMLAllCollectionCustom.cpp:
(WebCore::V8HTMLAllCollection::namedPropertyGetter):
* bindings/v8/custom/V8HTMLCollectionCustom.cpp:
(WebCore::V8HTMLCollection::namedPropertyGetter):
* bindings/v8/custom/V8NamedNodeMapCustom.cpp:
(WebCore::V8NamedNodeMap::namedPropertyGetter):
* bindings/v8/custom/V8StorageCustom.cpp:
(WebCore::storageSetter):

LayoutTests: 

Test how document.all behaves when you change its prototype chain.

* http/tests/security/document-all-expected.txt: Added.
* http/tests/security/document-all.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (95488 => 95489)


--- trunk/LayoutTests/ChangeLog	2011-09-19 22:56:22 UTC (rev 95488)
+++ trunk/LayoutTests/ChangeLog	2011-09-19 22:57:30 UTC (rev 95489)
@@ -1,5 +1,17 @@
 2011-09-19  Adam Barth  <aba...@webkit.org>
 
+        [V8] document.all gets confused about its prototype chain
+        https://bugs.webkit.org/show_bug.cgi?id=68393
+
+        Reviewed by Eric Seidel.
+
+        Test how document.all behaves when you change its prototype chain.
+
+        * http/tests/security/document-all-expected.txt: Added.
+        * http/tests/security/document-all.html: Added.
+
+2011-09-19  Adam Barth  <aba...@webkit.org>
+
         Named property confusion with __proto__
         https://bugs.webkit.org/show_bug.cgi?id=68221
 

Added: trunk/LayoutTests/http/tests/security/document-all-expected.txt (0 => 95489)


--- trunk/LayoutTests/http/tests/security/document-all-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/http/tests/security/document-all-expected.txt	2011-09-19 22:57:30 UTC (rev 95489)
@@ -0,0 +1,2 @@
+CONSOLE MESSAGE: line 1: Uncaught TypeError: Illegal constructor
+

Added: trunk/LayoutTests/http/tests/security/document-all.html (0 => 95489)


--- trunk/LayoutTests/http/tests/security/document-all.html	                        (rev 0)
+++ trunk/LayoutTests/http/tests/security/document-all.html	2011-09-19 22:57:30 UTC (rev 95489)
@@ -0,0 +1,28 @@
+<script>
+if (window.layoutTestController) {
+    layoutTestController.dumpAsText();
+    layoutTestController.waitUntilDone();
+}
+
+window._onload_ = function()
+{
+    frame = document.body.appendChild(document.createElement("iframe"));
+    frame.src = ""
+    frame._onload_ = function() {
+        frame._onload_ = null;
+
+        frame.contentWindow[0].location = "data:text/html,<script>(" + function() {
+
+            setTimeout(function() {
+                if (window.layoutTestController)
+                    layoutTestController.notifyDone();
+            }, 0);
+
+            window.name = "alert";
+            obj = document.all;
+            obj.__proto__ = parent;
+            alert(obj.alert.constructor("return document.body.innerHTML")());
+        } + ")()</scr" + "ipt>";
+    }
+}
+</script>

Modified: trunk/Source/WebCore/ChangeLog (95488 => 95489)


--- trunk/Source/WebCore/ChangeLog	2011-09-19 22:56:22 UTC (rev 95488)
+++ trunk/Source/WebCore/ChangeLog	2011-09-19 22:57:30 UTC (rev 95489)
@@ -1,5 +1,32 @@
 2011-09-19  Adam Barth  <aba...@webkit.org>
 
+        [V8] document.all gets confused about its prototype chain
+        https://bugs.webkit.org/show_bug.cgi?id=68393
+
+        Reviewed by Eric Seidel.
+
+        GetRealNamedPropertyInPrototypeChain doesn't call interceptors, so it's
+        not a good idea to use its return value.  It turns out that all the
+        callers of the API only cared about whether it returns a null handle.
+
+        Test: http/tests/security/document-all.html
+
+        * bindings/v8/V8Collection.h:
+        (WebCore::collectionNamedPropertyGetter):
+        * bindings/v8/custom/V8DOMStringMapCustom.cpp:
+        (WebCore::V8DOMStringMap::namedPropertyDeleter):
+        (WebCore::V8DOMStringMap::namedPropertySetter):
+        * bindings/v8/custom/V8HTMLAllCollectionCustom.cpp:
+        (WebCore::V8HTMLAllCollection::namedPropertyGetter):
+        * bindings/v8/custom/V8HTMLCollectionCustom.cpp:
+        (WebCore::V8HTMLCollection::namedPropertyGetter):
+        * bindings/v8/custom/V8NamedNodeMapCustom.cpp:
+        (WebCore::V8NamedNodeMap::namedPropertyGetter):
+        * bindings/v8/custom/V8StorageCustom.cpp:
+        (WebCore::storageSetter):
+
+2011-09-19  Adam Barth  <aba...@webkit.org>
+
         Named property confusion with __proto__
         https://bugs.webkit.org/show_bug.cgi?id=68221
 

Modified: trunk/Source/WebCore/bindings/v8/V8Collection.h (95488 => 95489)


--- trunk/Source/WebCore/bindings/v8/V8Collection.h	2011-09-19 22:56:22 UTC (rev 95488)
+++ trunk/Source/WebCore/bindings/v8/V8Collection.h	2011-09-19 22:57:30 UTC (rev 95489)
@@ -73,15 +73,11 @@
 // A template of named property accessor of collections.
 template<class Collection, class ItemType> static v8::Handle<v8::Value> collectionNamedPropertyGetter(v8::Local<v8::String> name, const v8::AccessorInfo& info)
 {
-    v8::Handle<v8::Value> value = info.Holder()->GetRealNamedPropertyInPrototypeChain(name);
-
-    if (!value.IsEmpty())
-        return value;
-
-    // Search local callback properties next to find IDL defined
-    // properties.
+    if (!info.Holder()->GetRealNamedPropertyInPrototypeChain(name).IsEmpty())
+        return notHandledByInterceptor();
     if (info.Holder()->HasRealNamedCallbackProperty(name))
         return notHandledByInterceptor();
+
     return getNamedPropertyOfCollection<Collection, ItemType>(name, info.Holder());
 }
 

Modified: trunk/Source/WebCore/bindings/v8/custom/V8DOMStringMapCustom.cpp (95488 => 95489)


--- trunk/Source/WebCore/bindings/v8/custom/V8DOMStringMapCustom.cpp	2011-09-19 22:56:22 UTC (rev 95488)
+++ trunk/Source/WebCore/bindings/v8/custom/V8DOMStringMapCustom.cpp	2011-09-19 22:57:30 UTC (rev 95489)
@@ -66,8 +66,8 @@
 v8::Handle<v8::Boolean> V8DOMStringMap::namedPropertyDeleter(v8::Local<v8::String> name, const v8::AccessorInfo& info)
 {
     INC_STATS("DOM.DOMStringMap.NamedPropertyDeleter");
-    v8::Handle<v8::Value> value = info.Holder()->GetRealNamedPropertyInPrototypeChain(name);
-    if (!value.IsEmpty())
+
+    if (!info.Holder()->GetRealNamedPropertyInPrototypeChain(name).IsEmpty())
         return v8::False();
     if (info.Holder()->HasRealNamedCallbackProperty(name))
         return v8::False();
@@ -82,8 +82,8 @@
 v8::Handle<v8::Value> V8DOMStringMap::namedPropertySetter(v8::Local<v8::String> name, v8::Local<v8::Value> value, const v8::AccessorInfo& info)
 {
     INC_STATS("DOM.DOMStringMap.NamedPropertySetter");
-    v8::Handle<v8::Value> property = info.Holder()->GetRealNamedPropertyInPrototypeChain(name);
-    if (!property.IsEmpty())
+
+    if (!info.Holder()->GetRealNamedPropertyInPrototypeChain(name).IsEmpty())
         return value;
     if (info.Holder()->HasRealNamedCallbackProperty(name))
         return value;

Modified: trunk/Source/WebCore/bindings/v8/custom/V8HTMLAllCollectionCustom.cpp (95488 => 95489)


--- trunk/Source/WebCore/bindings/v8/custom/V8HTMLAllCollectionCustom.cpp	2011-09-19 22:56:22 UTC (rev 95488)
+++ trunk/Source/WebCore/bindings/v8/custom/V8HTMLAllCollectionCustom.cpp	2011-09-19 22:57:30 UTC (rev 95489)
@@ -74,18 +74,12 @@
 v8::Handle<v8::Value> V8HTMLAllCollection::namedPropertyGetter(v8::Local<v8::String> name, const v8::AccessorInfo& info)
 {
     INC_STATS("DOM.HTMLAllCollection.NamedPropertyGetter");
-    // Search the prototype chain first.
-    v8::Handle<v8::Value> value = info.Holder()->GetRealNamedPropertyInPrototypeChain(name);
 
-    if (!value.IsEmpty())
-        return value;
-
-    // Search local callback properties next to find IDL defined
-    // properties.
+    if (!info.Holder()->GetRealNamedPropertyInPrototypeChain(name).IsEmpty())
+        return notHandledByInterceptor();
     if (info.Holder()->HasRealNamedCallbackProperty(name))
-        return v8::Handle<v8::Value>();
+        return notHandledByInterceptor();
 
-    // Finally, search the DOM structure.
     HTMLAllCollection* imp = V8HTMLAllCollection::toNative(info.Holder());
     return getNamedItems(imp, v8StringToAtomicWebCoreString(name));
 }

Modified: trunk/Source/WebCore/bindings/v8/custom/V8HTMLCollectionCustom.cpp (95488 => 95489)


--- trunk/Source/WebCore/bindings/v8/custom/V8HTMLCollectionCustom.cpp	2011-09-19 22:56:22 UTC (rev 95488)
+++ trunk/Source/WebCore/bindings/v8/custom/V8HTMLCollectionCustom.cpp	2011-09-19 22:57:30 UTC (rev 95489)
@@ -77,18 +77,12 @@
 v8::Handle<v8::Value> V8HTMLCollection::namedPropertyGetter(v8::Local<v8::String> name, const v8::AccessorInfo& info)
 {
     INC_STATS("DOM.HTMLCollection.NamedPropertyGetter");
-    // Search the prototype chain first.
-    v8::Handle<v8::Value> value = info.Holder()->GetRealNamedPropertyInPrototypeChain(name);
 
-    if (!value.IsEmpty())
-        return value;
-
-    // Search local callback properties next to find IDL defined
-    // properties.
+    if (!info.Holder()->GetRealNamedPropertyInPrototypeChain(name).IsEmpty())
+        return notHandledByInterceptor();
     if (info.Holder()->HasRealNamedCallbackProperty(name))
-        return v8::Handle<v8::Value>();
+        return notHandledByInterceptor();
 
-    // Finally, search the DOM structure.
     HTMLCollection* imp = V8HTMLCollection::toNative(info.Holder());
     return getNamedItems(imp, v8StringToAtomicWebCoreString(name));
 }

Modified: trunk/Source/WebCore/bindings/v8/custom/V8NamedNodeMapCustom.cpp (95488 => 95489)


--- trunk/Source/WebCore/bindings/v8/custom/V8NamedNodeMapCustom.cpp	2011-09-19 22:56:22 UTC (rev 95488)
+++ trunk/Source/WebCore/bindings/v8/custom/V8NamedNodeMapCustom.cpp	2011-09-19 22:57:30 UTC (rev 95489)
@@ -57,16 +57,12 @@
 v8::Handle<v8::Value> V8NamedNodeMap::namedPropertyGetter(v8::Local<v8::String> name, const v8::AccessorInfo& info)
 {
     INC_STATS("DOM.NamedNodeMap.NamedPropertyGetter");
-    // Search the prototype chain first.
-    v8::Handle<v8::Value> value = info.Holder()->GetRealNamedPropertyInPrototypeChain(name);
-    if (!value.IsEmpty())
-        return value;
 
-    // Then look for IDL defined properties on the object itself.
+    if (!info.Holder()->GetRealNamedPropertyInPrototypeChain(name).IsEmpty())
+        return notHandledByInterceptor();
     if (info.Holder()->HasRealNamedCallbackProperty(name))
         return notHandledByInterceptor();
 
-    // Finally, search the DOM.
     NamedNodeMap* imp = V8NamedNodeMap::toNative(info.Holder());
     RefPtr<Node> result = imp->getNamedItem(toWebCoreString(name));
     if (!result)

Modified: trunk/Source/WebCore/bindings/v8/custom/V8StorageCustom.cpp (95488 => 95489)


--- trunk/Source/WebCore/bindings/v8/custom/V8StorageCustom.cpp	2011-09-19 22:56:22 UTC (rev 95488)
+++ trunk/Source/WebCore/bindings/v8/custom/V8StorageCustom.cpp	2011-09-19 22:57:30 UTC (rev 95489)
@@ -102,8 +102,7 @@
     if (name == "length")
         return v8Value;
 
-    v8::Handle<v8::Value> prototypeValue = info.Holder()->GetRealNamedPropertyInPrototypeChain(v8Name);
-    if (!prototypeValue.IsEmpty())
+    if (!info.Holder()->GetRealNamedPropertyInPrototypeChain(v8Name).IsEmpty())
         return notHandledByInterceptor();
 
     ExceptionCode ec = 0;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to