Changeset 245249 in webkit

Timestamp:

May 13, 2019 1:52:04 PM (5 years ago)

Author:

Tadeu Zagallo

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

Location:

trunk

Files:

• JSTests/ChangeLog (modified) (1 diff)
• JSTests/stress/proxy-spread.js (added)
• Source/JavaScriptCore/ChangeLog (modified) (1 diff)
• Source/JavaScriptCore/runtime/JSObject.cpp (modified) (2 diffs)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/bindings/js/JSLocationCustom.cpp (modified) (1 diff)

trunk/JSTests/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/JavaScriptCore/runtime/JSObject.cpp

@@ -3442 +3442 @@
 {
     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;
 
@@ -3489 +3493 @@
         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;
 }

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebCore/bindings/js/JSLocationCustom.cpp

@@ -74 +74 @@
     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))
-        return true;
-    return JSObject::getOwnPropertySlot(object, state, propertyName, slot);
+    bool result = getOwnPropertySlotCommon(*thisObject, *state, propertyName, slot);
+    EXCEPTION_ASSERT(!scope.exception() || !result);
+    RETURN_IF_EXCEPTION(scope, false);
+    if (result)
+        return true;
+    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))
-        return true;
-    return JSObject::getOwnPropertySlotByIndex(object, 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;
+    RELEASE_AND_RETURN(scope, JSObject::getOwnPropertySlotByIndex(object, state, index, slot));
 }