Crash when passing Symbol to NPAPI plugin objects
authorutatane.tea@gmail.com <utatane.tea@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 9 Jun 2015 18:31:02 +0000 (18:31 +0000)
committerutatane.tea@gmail.com <utatane.tea@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 9 Jun 2015 18:31:02 +0000 (18:31 +0000)
https://bugs.webkit.org/show_bug.cgi?id=145798

Reviewed by Darin Adler.

Source/WebCore:

Test: plugins/npruntime/script-object-with-symbols.html

For C bridge APIs, we add guards for symbols.
This is the same to the existing guards in Objective-C APIs.

* bridge/c/c_class.cpp:
(JSC::Bindings::CClass::methodNamed):
(JSC::Bindings::CClass::fieldNamed):
* bridge/objc/objc_class.mm:
(JSC::Bindings::ObjcClass::methodNamed):
(JSC::Bindings::ObjcClass::fieldNamed):
(JSC::Bindings::ObjcClass::fallbackObject):

Source/WebKit2:

When the symbol is passed, `propertyName.publicName()` becomes nullptr.
So dereferencing it causes null dereference errors.
At first, this bug appears in the https://bugs.webkit.org/show_bug.cgi?id=145556,
plugin[@@toStringTag] ("string" + plugin) causes SEGV in plugins/embed-inside-object.html test.

This patch avoids it by early returning when the symbols are passed.
Methods for symbols are not implemented in the NPObject side, so it works correctly.

* WebProcess/Plugins/Netscape/JSNPObject.cpp:
(WebKit::npIdentifierFromIdentifier):
(WebKit::JSNPObject::callMethod):
(WebKit::JSNPObject::getOwnPropertySlot):
(WebKit::JSNPObject::put):
(WebKit::JSNPObject::deleteProperty):
(WebKit::JSNPObject::propertyGetter):
(WebKit::JSNPObject::methodGetter):

LayoutTests:

* plugins/npruntime/script-object-with-symbols-expected.txt: Added.
* plugins/npruntime/script-object-with-symbols.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@185369 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/plugins/npruntime/script-object-with-symbols-expected.txt [new file with mode: 0644]
LayoutTests/plugins/npruntime/script-object-with-symbols.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/bridge/c/c_class.cpp
Source/WebCore/bridge/objc/objc_class.mm
Source/WebKit2/ChangeLog
Source/WebKit2/WebProcess/Plugins/Netscape/JSNPObject.cpp

index d6d4eeb..0ea69ec 100644 (file)
@@ -1,3 +1,13 @@
+2015-06-09  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        Crash when passing Symbol to NPAPI plugin objects
+        https://bugs.webkit.org/show_bug.cgi?id=145798
+
+        Reviewed by Darin Adler.
+
+        * plugins/npruntime/script-object-with-symbols-expected.txt: Added.
+        * plugins/npruntime/script-object-with-symbols.html: Added.
+
 2015-06-09  Daniel Bates  <dabates@apple.com>
 
         Update iOS TestExpectations files 
diff --git a/LayoutTests/plugins/npruntime/script-object-with-symbols-expected.txt b/LayoutTests/plugins/npruntime/script-object-with-symbols-expected.txt
new file mode 100644 (file)
index 0000000..bf80975
--- /dev/null
@@ -0,0 +1,7 @@
+This tests that NPAPI plugin object can accept ES6 symbols without crash.
+SUCCESS
+
+
+
+
+
diff --git a/LayoutTests/plugins/npruntime/script-object-with-symbols.html b/LayoutTests/plugins/npruntime/script-object-with-symbols.html
new file mode 100644 (file)
index 0000000..95e23ce
--- /dev/null
@@ -0,0 +1,48 @@
+<html>
+<script>
+function runTest()
+{
+    if (window.testRunner)
+        testRunner.dumpAsText();
+
+    var plugin = document.getElementById("testPlugin");
+
+    var symbol = Symbol("Cappuccino");
+
+    // Put.
+    plugin[symbol] = 20;
+
+    // Get.
+    var result = plugin[symbol];
+
+    if (result !== 20)
+        return;
+
+    // Missing Get.
+    var missing = plugin[Symbol("Cocoa")];
+
+    if (missing !== undefined)
+        return;
+
+    // Delete an existing property.
+    if (!(delete plugin[symbol]))
+        return;
+
+    if (plugin[symbol] !== undefined)
+        return;
+
+    // Delete an non-exisitng property.
+    if (!(delete plugin[Symbol("Cappuccino")]))
+        return;
+
+    document.getElementById("result").innerHTML = "SUCCESS";
+}
+</script>
+
+<body onload="runTest();">
+<pre>
+This tests that NPAPI plugin object can accept ES6 symbols without crash.
+<div id="result">FAILURE</div>
+<embed id="testPlugin" type="application/x-webkit-test-netscape" width="200" height="200"></embed>
+</body>
+</html>
index 5daa4d4..42144a8 100644 (file)
@@ -1,3 +1,23 @@
+2015-06-09  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        Crash when passing Symbol to NPAPI plugin objects
+        https://bugs.webkit.org/show_bug.cgi?id=145798
+
+        Reviewed by Darin Adler.
+
+        Test: plugins/npruntime/script-object-with-symbols.html
+
+        For C bridge APIs, we add guards for symbols.
+        This is the same to the existing guards in Objective-C APIs.
+
+        * bridge/c/c_class.cpp:
+        (JSC::Bindings::CClass::methodNamed):
+        (JSC::Bindings::CClass::fieldNamed):
+        * bridge/objc/objc_class.mm:
+        (JSC::Bindings::ObjcClass::methodNamed):
+        (JSC::Bindings::ObjcClass::fieldNamed):
+        (JSC::Bindings::ObjcClass::fallbackObject):
+
 2015-06-09  Csaba Osztrogon├íc  <ossy@webkit.org>
 
         [cmake] Fix the style issues in cmake project files
index 9f8a6dd..5db2e01 100644 (file)
@@ -70,6 +70,8 @@ CClass* CClass::classForIsA(NPClass* isa)
 Method* CClass::methodNamed(PropertyName propertyName, Instance* instance) const
 {
     String name(propertyName.publicName());
+    if (name.isNull())
+        return nullptr;
     
     if (Method* method = m_methods.get(name.impl()))
         return method;
@@ -90,6 +92,8 @@ Method* CClass::methodNamed(PropertyName propertyName, Instance* instance) const
 Field* CClass::fieldNamed(PropertyName propertyName, Instance* instance) const
 {
     String name(propertyName.publicName());
+    if (name.isNull())
+        return nullptr;
     
     if (Field* field = m_fields.get(name.impl()))
         return field;
index 8c8eb20..f887b6c 100644 (file)
@@ -99,7 +99,7 @@ Method* ObjcClass::methodNamed(PropertyName propertyName, Instance*) const
 {
     String name(propertyName.publicName());
     if (name.isNull())
-        return 0;
+        return nullptr;
 
     if (Method* method = m_methodCache.get(name.impl()))
         return method;
@@ -150,7 +150,7 @@ Field* ObjcClass::fieldNamed(PropertyName propertyName, Instance* instance) cons
 {
     String name(propertyName.publicName());
     if (name.isNull())
-        return 0;
+        return nullptr;
 
     Field* field = m_fieldCache.get(name.impl());
     if (field)
@@ -239,6 +239,10 @@ JSValue ObjcClass::fallbackObject(ExecState* exec, Instance* instance, PropertyN
     
     if (![targetObject respondsToSelector:@selector(invokeUndefinedMethodFromWebScript:withArguments:)])
         return jsUndefined();
+
+    if (!propertyName.publicName())
+        return jsUndefined();
+
     return ObjcFallbackObjectImp::create(exec, exec->lexicalGlobalObject(), objcInstance, propertyName.publicName());
 }
 
index f7e2d01..9a194e4 100644 (file)
@@ -1,3 +1,27 @@
+2015-06-09  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        Crash when passing Symbol to NPAPI plugin objects
+        https://bugs.webkit.org/show_bug.cgi?id=145798
+
+        Reviewed by Darin Adler.
+
+        When the symbol is passed, `propertyName.publicName()` becomes nullptr.
+        So dereferencing it causes null dereference errors.
+        At first, this bug appears in the https://bugs.webkit.org/show_bug.cgi?id=145556,
+        plugin[@@toStringTag] ("string" + plugin) causes SEGV in plugins/embed-inside-object.html test.
+
+        This patch avoids it by early returning when the symbols are passed.
+        Methods for symbols are not implemented in the NPObject side, so it works correctly.
+
+        * WebProcess/Plugins/Netscape/JSNPObject.cpp:
+        (WebKit::npIdentifierFromIdentifier):
+        (WebKit::JSNPObject::callMethod):
+        (WebKit::JSNPObject::getOwnPropertySlot):
+        (WebKit::JSNPObject::put):
+        (WebKit::JSNPObject::deleteProperty):
+        (WebKit::JSNPObject::propertyGetter):
+        (WebKit::JSNPObject::methodGetter):
+
 2015-06-09  Carlos Garcia Campos  <cgarcia@igalia.com>
 
         [SOUP] Network Cache: Give more priority to reads over writes in IO WorkQueue
index fad2daa..cf5c71d 100644 (file)
@@ -50,8 +50,9 @@ namespace WebKit {
 static NPIdentifier npIdentifierFromIdentifier(PropertyName propertyName)
 {
     String name(propertyName.publicName());
+    // If the propertyName is Symbol.
     if (name.isNull())
-        return 0;
+        return nullptr;
     return static_cast<NPIdentifier>(IdentifierRep::get(name.utf8().data()));
 }
 
@@ -110,6 +111,10 @@ JSValue JSNPObject::callMethod(ExecState* exec, NPIdentifier methodName)
     if (!m_npObject)
         return throwInvalidAccessError(exec);
 
+    // If the propertyName is symbol.
+    if (!methodName)
+        return jsUndefined();
+
     size_t argumentCount = exec->argumentCount();
     Vector<NPVariant, 8> arguments(argumentCount);
 
@@ -268,6 +273,9 @@ bool JSNPObject::getOwnPropertySlot(JSObject* object, ExecState* exec, PropertyN
     }
     
     NPIdentifier npIdentifier = npIdentifierFromIdentifier(propertyName);
+    // If the propertyName is symbol.
+    if (!npIdentifier)
+        return false;
 
     // Calling NPClass::invoke will call into plug-in code, and there's no telling what the plug-in can do.
     // (including destroying the plug-in). Because of this, we make sure to keep the plug-in alive until 
@@ -299,6 +307,9 @@ void JSNPObject::put(JSCell* cell, ExecState* exec, PropertyName propertyName, J
     }
 
     NPIdentifier npIdentifier = npIdentifierFromIdentifier(propertyName);
+    // If the propertyName is symbol.
+    if (!npIdentifier)
+        return;
     
     if (!thisObject->m_npObject->_class->hasProperty || !thisObject->m_npObject->_class->hasProperty(thisObject->m_npObject, npIdentifier)) {
         // FIXME: Should we throw an exception here?
@@ -341,6 +352,11 @@ bool JSNPObject::deletePropertyByIndex(JSCell* cell, ExecState* exec, unsigned p
 bool JSNPObject::deleteProperty(ExecState* exec, NPIdentifier propertyName)
 {
     ASSERT_GC_OBJECT_INHERITS(this, info());
+
+    // If the propertyName is symbol.
+    if (!propertyName)
+        return false;
+
     if (!m_npObject) {
         throwInvalidAccessError(exec);
         return false;
@@ -440,6 +456,10 @@ EncodedJSValue JSNPObject::propertyGetter(ExecState* exec, JSObject* slotBase, E
     {
         JSLock::DropAllLocks dropAllLocks(JSDOMWindowBase::commonVM());
         NPIdentifier npIdentifier = npIdentifierFromIdentifier(propertyName);
+        // If the propertyName is symbol.
+        if (!npIdentifier)
+            return JSValue::encode(jsUndefined());
+
         returnValue = thisObj->m_npObject->_class->getProperty(thisObj->m_npObject, npIdentifier, &result);
         
         NPRuntimeObjectMap::moveGlobalExceptionToExecState(exec);
@@ -462,6 +482,10 @@ EncodedJSValue JSNPObject::methodGetter(ExecState* exec, JSObject* slotBase, Enc
         return JSValue::encode(throwInvalidAccessError(exec));
 
     NPIdentifier npIdentifier = npIdentifierFromIdentifier(propertyName);
+    // If the propertyName is symbol.
+    if (!npIdentifier)
+        return JSValue::encode(throwInvalidAccessError(exec));
+
     return JSValue::encode(JSNPMethod::create(exec, thisObj->globalObject(), propertyName.publicName(), npIdentifier));
 }