JSObject::getOwnPropertyDescriptor is missing an exception check
authortzagallo@apple.com <tzagallo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 13 May 2019 20:52:04 +0000 (20:52 +0000)
committertzagallo@apple.com <tzagallo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 13 May 2019 20:52:04 +0000 (20:52 +0000)
https://bugs.webkit.org/show_bug.cgi?id=197693
JSTests:

<rdar://problem/50441784>

Reviewed by Saam Barati.

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

Source/JavaScriptCore:

<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):

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

JSTests/ChangeLog
JSTests/stress/proxy-spread.js [new file with mode: 0644]
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/runtime/JSObject.cpp
Source/WebCore/ChangeLog
Source/WebCore/bindings/js/JSLocationCustom.cpp

index 2c83cfc..21ecceb 100644 (file)
@@ -1,3 +1,14 @@
+2019-05-13  Tadeu Zagallo  <tzagallo@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  <sbarati@apple.com>
 
         Call to JSToWasmICCallee::createStructure passes in wrong prototype value
diff --git a/JSTests/stress/proxy-spread.js b/JSTests/stress/proxy-spread.js
new file mode 100644 (file)
index 0000000..e8fd788
--- /dev/null
@@ -0,0 +1,3 @@
+function foo() {}
+let p = new Proxy(foo, {});
+let a = {...p};
index 56fb367..d26cb2b 100644 (file)
@@ -1,3 +1,17 @@
+2019-05-13  Tadeu Zagallo  <tzagallo@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  <ysuzuki@apple.com>
 
         [JSC] Compress miscelaneous JIT related data structures with Packed<>
index 8d3a0e7..a0c0b20 100644 (file)
@@ -3441,8 +3441,12 @@ static JSCustomGetterSetterFunction* getCustomGetterSetterFunctionForGetterSette
 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 @@ bool JSObject::getOwnPropertyDescriptor(ExecState* exec, PropertyName propertyNa
             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;
 }
 
index 8848d91..aa4c7a9 100644 (file)
@@ -1,3 +1,20 @@
+2019-05-13  Tadeu Zagallo  <tzagallo@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  <antti@apple.com>
 
         REGRESSION (r245208): compositing/shared-backing/sharing-bounds-non-clipping-shared-layer.html asserts
index b18f64e..13c2f65 100644 (file)
@@ -73,27 +73,37 @@ static bool getOwnPropertySlotCommon(JSLocation& thisObject, ExecState& state, P
 
     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)