Attachment 369719 Details for Bug 197693 – Patch

[patch] Patch

bug-197693-20190513112156.patch (text/plain), 6.94 KB, created by Tadeu Zagallo on 2019-05-13 02:21:57 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Tadeu Zagallo

Created: 2019-05-13 02:21:57 PDT

Size: 6.94 KB

patch

obsolete

>Subversion Revision: 244956
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index 7d960395584de6aa830c033c9f0c21d4162c62f6..d66e3dbde7c8dea3bcd167b1bbe4b320b898145a 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -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 NOBODY (OOPS!).
>+
>+        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-04  Tadeu Zagallo  <tzagallo@apple.com>
> 
>         TypedArrays should not store properties that are canonical numeric indices
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 87ff5149b122fb90b7ece5da662314ff61195f56..8e585157202c77be51f223496696b2518b0fabe1 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -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 NOBODY (OOPS!).
>+
>+        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-05  Wenson Hsieh  <wenson_hsieh@apple.com>
> 
>         fast/attachment/attachment-folder-icon.html is an Image Only failure on recent macOS builds
>diff --git a/Source/JavaScriptCore/runtime/JSObject.cpp b/Source/JavaScriptCore/runtime/JSObject.cpp
>index 8d3a0e703cd7da017e7044b45bf0c1bb4e8119fe..a0c0b20291eddf23e525b1deeaee38ac8a58ee61 100644
>--- a/Source/JavaScriptCore/runtime/JSObject.cpp
>+++ b/Source/JavaScriptCore/runtime/JSObject.cpp
>@@ -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;
> }
> 
>diff --git a/Source/WebCore/bindings/js/JSLocationCustom.cpp b/Source/WebCore/bindings/js/JSLocationCustom.cpp
>index b18f64e8ca546d15cd7c94dac38a85ccc44d3597..13c2f65017fa25b691ecfe0c93e904bc100e4cf5 100644
>--- a/Source/WebCore/bindings/js/JSLocationCustom.cpp
>+++ b/Source/WebCore/bindings/js/JSLocationCustom.cpp
>@@ -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)
>diff --git a/JSTests/ChangeLog b/JSTests/ChangeLog
>index c2316a6960e6628407f9cdb7dc3efee2b59cc899..8e8f2ea7f45682ec9d427e5c29a316ca8c12d2b2 100644
>--- a/JSTests/ChangeLog
>+++ b/JSTests/ChangeLog
>@@ -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 NOBODY (OOPS!).
>+
>+        * stress/proxy-spread.js: Added.
>+        (foo):
>+
> 2019-05-04  Tadeu Zagallo  <tzagallo@apple.com>
> 
>         TypedArrays should not store properties that are canonical numeric indices
>diff --git a/JSTests/stress/proxy-spread.js b/JSTests/stress/proxy-spread.js
>new file mode 100644
>index 0000000000000000000000000000000000000000..e8fd788c3a875b729a05f397d06b2f65ac8671fa
>--- /dev/null
>+++ b/JSTests/stress/proxy-spread.js
>@@ -0,0 +1,3 @@
>+function foo() {}
>+let p = new Proxy(foo, {});
>+let a = {...p};

Actions: View | Formatted Diff | Diff

Attachments on bug 197693: 369392 | 369399 | 369407 | 369421 | 369450 | 369719