From: abarth@webkit.org Date: Mon, 19 Sep 2011 22:57:30 +0000 (+0000) Subject: [V8] document.all gets confused about its prototype chain X-Git-Url: https://git.webkit.org/?p=WebKit.git;a=commitdiff_plain;h=dc55e1c7a8176af03be0c715b974f3226b3ed09a [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. git-svn-id: http://svn.webkit.org/repository/webkit/trunk@95489 268f45cc-cd09-0410-ab3c-d52691b4dbfc --- diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog index c24aeba..2258026 100644 --- a/LayoutTests/ChangeLog +++ b/LayoutTests/ChangeLog @@ -1,5 +1,17 @@ 2011-09-19 Adam Barth + [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 + Named property confusion with __proto__ https://bugs.webkit.org/show_bug.cgi?id=68221 diff --git a/LayoutTests/http/tests/security/document-all-expected.txt b/LayoutTests/http/tests/security/document-all-expected.txt new file mode 100644 index 0000000..89704c4 --- /dev/null +++ b/LayoutTests/http/tests/security/document-all-expected.txt @@ -0,0 +1,2 @@ +CONSOLE MESSAGE: line 1: Uncaught TypeError: Illegal constructor + diff --git a/LayoutTests/http/tests/security/document-all.html b/LayoutTests/http/tests/security/document-all.html new file mode 100644 index 0000000..d673ad4 --- /dev/null +++ b/LayoutTests/http/tests/security/document-all.html @@ -0,0 +1,28 @@ + diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog index 14d7f89..a613b1c 100644 --- a/Source/WebCore/ChangeLog +++ b/Source/WebCore/ChangeLog @@ -1,5 +1,32 @@ 2011-09-19 Adam Barth + [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 + Named property confusion with __proto__ https://bugs.webkit.org/show_bug.cgi?id=68221 diff --git a/Source/WebCore/bindings/v8/V8Collection.h b/Source/WebCore/bindings/v8/V8Collection.h index 1757c85..38a7c8c 100644 --- a/Source/WebCore/bindings/v8/V8Collection.h +++ b/Source/WebCore/bindings/v8/V8Collection.h @@ -73,15 +73,11 @@ template static v8::Handle getNamed // A template of named property accessor of collections. template static v8::Handle collectionNamedPropertyGetter(v8::Local name, const v8::AccessorInfo& info) { - v8::Handle 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(name, info.Holder()); } diff --git a/Source/WebCore/bindings/v8/custom/V8DOMStringMapCustom.cpp b/Source/WebCore/bindings/v8/custom/V8DOMStringMapCustom.cpp index 082be81..cebfae3 100644 --- a/Source/WebCore/bindings/v8/custom/V8DOMStringMapCustom.cpp +++ b/Source/WebCore/bindings/v8/custom/V8DOMStringMapCustom.cpp @@ -66,8 +66,8 @@ v8::Handle V8DOMStringMap::namedPropertyEnumerator(const v8::Accessor v8::Handle V8DOMStringMap::namedPropertyDeleter(v8::Local name, const v8::AccessorInfo& info) { INC_STATS("DOM.DOMStringMap.NamedPropertyDeleter"); - v8::Handle 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 V8DOMStringMap::namedPropertyDeleter(v8::Local V8DOMStringMap::namedPropertySetter(v8::Local name, v8::Local value, const v8::AccessorInfo& info) { INC_STATS("DOM.DOMStringMap.NamedPropertySetter"); - v8::Handle property = info.Holder()->GetRealNamedPropertyInPrototypeChain(name); - if (!property.IsEmpty()) + + if (!info.Holder()->GetRealNamedPropertyInPrototypeChain(name).IsEmpty()) return value; if (info.Holder()->HasRealNamedCallbackProperty(name)) return value; diff --git a/Source/WebCore/bindings/v8/custom/V8HTMLAllCollectionCustom.cpp b/Source/WebCore/bindings/v8/custom/V8HTMLAllCollectionCustom.cpp index ef428a9..a2b3cd2 100644 --- a/Source/WebCore/bindings/v8/custom/V8HTMLAllCollectionCustom.cpp +++ b/Source/WebCore/bindings/v8/custom/V8HTMLAllCollectionCustom.cpp @@ -74,18 +74,12 @@ static v8::Handle getItem(HTMLAllCollection* collection, v8::Handle V8HTMLAllCollection::namedPropertyGetter(v8::Local name, const v8::AccessorInfo& info) { INC_STATS("DOM.HTMLAllCollection.NamedPropertyGetter"); - // Search the prototype chain first. - v8::Handle 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(); + return notHandledByInterceptor(); - // Finally, search the DOM structure. HTMLAllCollection* imp = V8HTMLAllCollection::toNative(info.Holder()); return getNamedItems(imp, v8StringToAtomicWebCoreString(name)); } diff --git a/Source/WebCore/bindings/v8/custom/V8HTMLCollectionCustom.cpp b/Source/WebCore/bindings/v8/custom/V8HTMLCollectionCustom.cpp index 3d64edd..050a268 100644 --- a/Source/WebCore/bindings/v8/custom/V8HTMLCollectionCustom.cpp +++ b/Source/WebCore/bindings/v8/custom/V8HTMLCollectionCustom.cpp @@ -77,18 +77,12 @@ static v8::Handle getItem(HTMLCollection* collection, v8::Handle V8HTMLCollection::namedPropertyGetter(v8::Local name, const v8::AccessorInfo& info) { INC_STATS("DOM.HTMLCollection.NamedPropertyGetter"); - // Search the prototype chain first. - v8::Handle 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(); + return notHandledByInterceptor(); - // Finally, search the DOM structure. HTMLCollection* imp = V8HTMLCollection::toNative(info.Holder()); return getNamedItems(imp, v8StringToAtomicWebCoreString(name)); } diff --git a/Source/WebCore/bindings/v8/custom/V8NamedNodeMapCustom.cpp b/Source/WebCore/bindings/v8/custom/V8NamedNodeMapCustom.cpp index c9014c0..4782dcc 100644 --- a/Source/WebCore/bindings/v8/custom/V8NamedNodeMapCustom.cpp +++ b/Source/WebCore/bindings/v8/custom/V8NamedNodeMapCustom.cpp @@ -57,16 +57,12 @@ v8::Handle V8NamedNodeMap::indexedPropertyGetter(uint32_t index, cons v8::Handle V8NamedNodeMap::namedPropertyGetter(v8::Local name, const v8::AccessorInfo& info) { INC_STATS("DOM.NamedNodeMap.NamedPropertyGetter"); - // Search the prototype chain first. - v8::Handle 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 result = imp->getNamedItem(toWebCoreString(name)); if (!result) diff --git a/Source/WebCore/bindings/v8/custom/V8StorageCustom.cpp b/Source/WebCore/bindings/v8/custom/V8StorageCustom.cpp index d0a0d92..a252410 100644 --- a/Source/WebCore/bindings/v8/custom/V8StorageCustom.cpp +++ b/Source/WebCore/bindings/v8/custom/V8StorageCustom.cpp @@ -102,8 +102,7 @@ static v8::Handle storageSetter(v8::Local v8Name, v8::Loc if (name == "length") return v8Value; - v8::Handle prototypeValue = info.Holder()->GetRealNamedPropertyInPrototypeChain(v8Name); - if (!prototypeValue.IsEmpty()) + if (!info.Holder()->GetRealNamedPropertyInPrototypeChain(v8Name).IsEmpty()) return notHandledByInterceptor(); ExceptionCode ec = 0;