Title: [245249] trunk
Revision
245249
Author
tzaga...@apple.com
Date
2019-05-13 13:52:04 -0700 (Mon, 13 May 2019)

Log Message

JSObject::getOwnPropertyDescriptor is missing an exception check
https://bugs.webkit.org/show_bug.cgi?id=197693
JSTests:

<rdar://problem/50441784>

Reviewed by Saam Barati.

* stress/proxy-spread.js: Added.
(foo):

Source/_javascript_Core:

<rdar://problem/50441784>

Reviewed by Saam Barati.

The method table call to getOwnPropertySlot might throw, and JSObject::getOwnPropertyDescriptor
must handle the exception before calling PropertySlot::getValue, which can also throw.

* runtime/JSObject.cpp:
(JSC::JSObject::getOwnPropertyDescriptor):

Source/WebCore:

Reviewed by Saam Barati.

JSObject::getOwnPropertyDescriptor assumes that getOwnPropertySlot returns false
if an exception is thrown, but that was not true for JSLocation::getOwnPropertySlotCommon.

This is already covered by http/tests/security/cross-frame-access-getOwnPropertyDescriptor.html

* bindings/js/JSLocationCustom.cpp:
(WebCore::getOwnPropertySlotCommon):
(WebCore::JSLocation::getOwnPropertySlot):
(WebCore::JSLocation::getOwnPropertySlotByIndex):

Modified Paths

Added Paths

Diff

Modified: trunk/JSTests/ChangeLog (245248 => 245249)


--- trunk/JSTests/ChangeLog	2019-05-13 20:20:29 UTC (rev 245248)
+++ trunk/JSTests/ChangeLog	2019-05-13 20:52:04 UTC (rev 245249)
@@ -1,3 +1,14 @@
+2019-05-13  Tadeu Zagallo  <tzaga...@apple.com>
+
+        JSObject::getOwnPropertyDescriptor is missing an exception check
+        https://bugs.webkit.org/show_bug.cgi?id=197693
+        <rdar://problem/50441784>
+
+        Reviewed by Saam Barati.
+
+        * stress/proxy-spread.js: Added.
+        (foo):
+
 2019-05-10  Saam barati  <sbar...@apple.com>
 
         Call to JSToWasmICCallee::createStructure passes in wrong prototype value

Added: trunk/JSTests/stress/proxy-spread.js (0 => 245249)


--- trunk/JSTests/stress/proxy-spread.js	                        (rev 0)
+++ trunk/JSTests/stress/proxy-spread.js	2019-05-13 20:52:04 UTC (rev 245249)
@@ -0,0 +1,3 @@
+function foo() {}
+let p = new Proxy(foo, {});
+let a = {...p};

Modified: trunk/Source/_javascript_Core/ChangeLog (245248 => 245249)


--- trunk/Source/_javascript_Core/ChangeLog	2019-05-13 20:20:29 UTC (rev 245248)
+++ trunk/Source/_javascript_Core/ChangeLog	2019-05-13 20:52:04 UTC (rev 245249)
@@ -1,3 +1,17 @@
+2019-05-13  Tadeu Zagallo  <tzaga...@apple.com>
+
+        JSObject::getOwnPropertyDescriptor is missing an exception check
+        https://bugs.webkit.org/show_bug.cgi?id=197693
+        <rdar://problem/50441784>
+
+        Reviewed by Saam Barati.
+
+        The method table call to getOwnPropertySlot might throw, and JSObject::getOwnPropertyDescriptor
+        must handle the exception before calling PropertySlot::getValue, which can also throw.
+
+        * runtime/JSObject.cpp:
+        (JSC::JSObject::getOwnPropertyDescriptor):
+
 2019-05-13  Yusuke Suzuki  <ysuz...@apple.com>
 
         [JSC] Compress miscelaneous JIT related data structures with Packed<>

Modified: trunk/Source/_javascript_Core/runtime/JSObject.cpp (245248 => 245249)


--- trunk/Source/_javascript_Core/runtime/JSObject.cpp	2019-05-13 20:20:29 UTC (rev 245248)
+++ trunk/Source/_javascript_Core/runtime/JSObject.cpp	2019-05-13 20:52:04 UTC (rev 245249)
@@ -3441,8 +3441,12 @@
 bool JSObject::getOwnPropertyDescriptor(ExecState* exec, PropertyName propertyName, PropertyDescriptor& descriptor)
 {
     VM& vm = exec->vm();
+    auto scope = DECLARE_THROW_SCOPE(vm);
     JSC::PropertySlot slot(this, PropertySlot::InternalMethodType::GetOwnProperty);
-    if (!methodTable(vm)->getOwnPropertySlot(this, exec, propertyName, slot))
+
+    bool result = methodTable(vm)->getOwnPropertySlot(this, exec, propertyName, slot);
+    EXCEPTION_ASSERT(!scope.exception() || !result);
+    if (!result)
         return false;
 
     // DebuggerScope::getOwnPropertySlot() (and possibly others) may return attributes from the prototype chain
@@ -3488,8 +3492,12 @@
             descriptor.setGetter(getCustomGetterSetterFunctionForGetterSetter(exec, propertyName, getterSetter, JSCustomGetterSetterFunction::Type::Getter));
         if (getterSetter->setter())
             descriptor.setSetter(getCustomGetterSetterFunctionForGetterSetter(exec, propertyName, getterSetter, JSCustomGetterSetterFunction::Type::Setter));
-    } else
-        descriptor.setDescriptor(slot.getValue(exec, propertyName), slot.attributes());
+    } else {
+        JSValue value = slot.getValue(exec, propertyName);
+        RETURN_IF_EXCEPTION(scope, false);
+        descriptor.setDescriptor(value, slot.attributes());
+    }
+
     return true;
 }
 

Modified: trunk/Source/WebCore/ChangeLog (245248 => 245249)


--- trunk/Source/WebCore/ChangeLog	2019-05-13 20:20:29 UTC (rev 245248)
+++ trunk/Source/WebCore/ChangeLog	2019-05-13 20:52:04 UTC (rev 245249)
@@ -1,3 +1,20 @@
+2019-05-13  Tadeu Zagallo  <tzaga...@apple.com>
+
+        JSObject::getOwnPropertyDescriptor is missing an exception check
+        https://bugs.webkit.org/show_bug.cgi?id=197693
+
+        Reviewed by Saam Barati.
+
+        JSObject::getOwnPropertyDescriptor assumes that getOwnPropertySlot returns false
+        if an exception is thrown, but that was not true for JSLocation::getOwnPropertySlotCommon.
+
+        This is already covered by http/tests/security/cross-frame-access-getOwnPropertyDescriptor.html
+
+        * bindings/js/JSLocationCustom.cpp:
+        (WebCore::getOwnPropertySlotCommon):
+        (WebCore::JSLocation::getOwnPropertySlot):
+        (WebCore::JSLocation::getOwnPropertySlotByIndex):
+
 2019-05-13  Antti Koivisto  <an...@apple.com>
 
         REGRESSION (r245208): compositing/shared-backing/sharing-bounds-non-clipping-shared-layer.html asserts

Modified: trunk/Source/WebCore/bindings/js/JSLocationCustom.cpp (245248 => 245249)


--- trunk/Source/WebCore/bindings/js/JSLocationCustom.cpp	2019-05-13 20:20:29 UTC (rev 245248)
+++ trunk/Source/WebCore/bindings/js/JSLocationCustom.cpp	2019-05-13 20:52:04 UTC (rev 245249)
@@ -73,27 +73,37 @@
 
     throwSecurityError(state, scope, message);
     slot.setUndefined();
-    return true;
+    return false;
 }
 
 bool JSLocation::getOwnPropertySlot(JSObject* object, ExecState* state, PropertyName propertyName, PropertySlot& slot)
 {
+    VM& vm = state->vm();
+    auto scope = DECLARE_THROW_SCOPE(vm);
     auto* thisObject = jsCast<JSLocation*>(object);
     ASSERT_GC_OBJECT_INHERITS(thisObject, info());
 
-    if (getOwnPropertySlotCommon(*thisObject, *state, propertyName, slot))
+    bool result = getOwnPropertySlotCommon(*thisObject, *state, propertyName, slot);
+    EXCEPTION_ASSERT(!scope.exception() || !result);
+    RETURN_IF_EXCEPTION(scope, false);
+    if (result)
         return true;
-    return JSObject::getOwnPropertySlot(object, state, propertyName, slot);
+    RELEASE_AND_RETURN(scope, JSObject::getOwnPropertySlot(object, state, propertyName, slot));
 }
 
 bool JSLocation::getOwnPropertySlotByIndex(JSObject* object, ExecState* state, unsigned index, PropertySlot& slot)
 {
+    VM& vm = state->vm();
+    auto scope = DECLARE_THROW_SCOPE(vm);
     auto* thisObject = jsCast<JSLocation*>(object);
     ASSERT_GC_OBJECT_INHERITS(thisObject, info());
 
-    if (getOwnPropertySlotCommon(*thisObject, *state, Identifier::from(state, index), slot))
+    bool result = getOwnPropertySlotCommon(*thisObject, *state, Identifier::from(state, index), slot);
+    EXCEPTION_ASSERT(!scope.exception() || !result);
+    RETURN_IF_EXCEPTION(scope, false);
+    if (result)
         return true;
-    return JSObject::getOwnPropertySlotByIndex(object, state, index, slot);
+    RELEASE_AND_RETURN(scope, JSObject::getOwnPropertySlotByIndex(object, state, index, slot));
 }
 
 static bool putCommon(JSLocation& thisObject, ExecState& state, PropertyName propertyName)
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to