Changeset 251274 in webkit

Timestamp:

Oct 17, 2019, 9:35:28 PM (6 years ago)

Author:

mark.lam@apple.com

Message:

Add missing checks after calls to the sameValue() JSValue comparator.
​https://bugs.webkit.org/show_bug.cgi?id=203126
<rdar://problem/56366561>

Reviewed by Saam Barati.

JSTests:

• stress/validate-exception-check-in-proxy-object-put.js: Added.

Source/JavaScriptCore:

• runtime/JSFunction.cpp:

(JSC::JSFunction::defineOwnProperty):

• runtime/JSObject.cpp:

(JSC::JSObject::defineOwnIndexedProperty):
(JSC::validateAndApplyPropertyDescriptor):

• runtime/PropertyDescriptor.cpp:

(JSC::PropertyDescriptor::equalTo const):

• runtime/ProxyObject.cpp:

(JSC::performProxyGet):
(JSC::ProxyObject::performPut):
(JSC::ProxyObject::performSetPrototype):
(JSC::ProxyObject::performGetPrototype):

• runtime/RegExpObject.cpp:

(JSC::RegExpObject::defineOwnProperty):

Location:

trunk

Files:

• JSTests/ChangeLog (modified) (1 diff)
• JSTests/stress/validate-exception-check-in-proxy-object-put.js (added)
• Source/JavaScriptCore/ChangeLog (modified) (1 diff)
• Source/JavaScriptCore/runtime/JSFunction.cpp (modified) (1 diff)
• Source/JavaScriptCore/runtime/JSObject.cpp (modified) (2 diffs)
• Source/JavaScriptCore/runtime/PropertyDescriptor.cpp (modified) (2 diffs)
• Source/JavaScriptCore/runtime/ProxyObject.cpp (modified) (4 diffs)
• Source/JavaScriptCore/runtime/RegExpObject.cpp (modified) (2 diffs)

trunk/JSTests/ChangeLog

@@ +1 @@
+2019-10-17  Mark Lam  <mark.lam@apple.com>
+
+        Add missing checks after calls to the sameValue() JSValue comparator.
+        https://bugs.webkit.org/show_bug.cgi?id=203126
+        <rdar://problem/56366561>
+
+        Reviewed by Saam Barati.
+
+        * stress/validate-exception-check-in-proxy-object-put.js: Added.
+
 2019-10-17  Saam Barati  <sbarati@apple.com>
 

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2019-10-17  Mark Lam  <mark.lam@apple.com>
+
+        Add missing checks after calls to the sameValue() JSValue comparator.
+        https://bugs.webkit.org/show_bug.cgi?id=203126
+        <rdar://problem/56366561>
+
+        Reviewed by Saam Barati.
+
+        * runtime/JSFunction.cpp:
+        (JSC::JSFunction::defineOwnProperty):
+        * runtime/JSObject.cpp:
+        (JSC::JSObject::defineOwnIndexedProperty):
+        (JSC::validateAndApplyPropertyDescriptor):
+        * runtime/PropertyDescriptor.cpp:
+        (JSC::PropertyDescriptor::equalTo const):
+        * runtime/ProxyObject.cpp:
+        (JSC::performProxyGet):
+        (JSC::ProxyObject::performPut):
+        (JSC::ProxyObject::performSetPrototype):
+        (JSC::ProxyObject::performGetPrototype):
+        * runtime/RegExpObject.cpp:
+        (JSC::RegExpObject::defineOwnProperty):
+
 2019-10-17  Saam Barati  <sbarati@apple.com>
 

trunk/Source/JavaScriptCore/runtime/JSFunction.cpp

@@ -592 +592 @@
             RELEASE_AND_RETURN(scope, Base::defineOwnProperty(object, exec, propertyName, descriptor, throwException));
 
-        valueCheck = !descriptor.value() || sameValue(exec, descriptor.value(), retrieveArguments(exec, thisObject));
+        valueCheck = !descriptor.value();
+        if (!valueCheck) {
+            valueCheck = sameValue(exec, descriptor.value(), retrieveArguments(exec, thisObject));
+            RETURN_IF_EXCEPTION(scope, false);
+        }
     } else if (propertyName == vm.propertyNames->caller) {
         if (!thisObject->jsExecutable()->hasCallerAndArgumentsProperties())
             RELEASE_AND_RETURN(scope, Base::defineOwnProperty(object, exec, propertyName, descriptor, throwException));
 
-        valueCheck = !descriptor.value() || sameValue(exec, descriptor.value(), retrieveCallerFunction(exec, thisObject));
+        valueCheck = !descriptor.value();
+        if (!valueCheck) {
+            valueCheck = sameValue(exec, descriptor.value(), retrieveCallerFunction(exec, thisObject));
+            RETURN_IF_EXCEPTION(scope, false);
+        }
     } else {
         thisObject->reifyLazyPropertyIfNeeded(vm, exec, propertyName);

trunk/Source/JavaScriptCore/runtime/JSObject.cpp

@@ -2684 +2684 @@
                 // 10.a.ii. If the [[Writable]] field of current is false, then
                 // 10.a.ii.1. Reject, if the [[Value]] field of Desc is present and SameValue(Desc.[[Value]], current.[[Value]]) is false.
-                if (descriptor.value() && !sameValue(exec, descriptor.value(), current.value()))
-                    return typeError(exec, scope, throwException, ReadonlyPropertyChangeError);
+                if (descriptor.value()) {
+                    bool isSame = sameValue(exec, descriptor.value(), current.value());
+                    RETURN_IF_EXCEPTION(scope, false);
+                    if (!isSame)
+                        return typeError(exec, scope, throwException, ReadonlyPropertyChangeError);
+                }
             }
             // 10.b. else, the [[Configurable]] field of current is true, so any change is acceptable.
@@ -3642 +3646 @@
                 return typeError(exec, scope, throwException, UnconfigurablePropertyChangeWritabilityError);
             if (!current.writable()) {
-                if (descriptor.value() && !sameValue(exec, current.value(), descriptor.value()))
-                    return typeError(exec, scope, throwException, ReadonlyPropertyChangeError);
+                if (descriptor.value()) {
+                    bool isSame = sameValue(exec, current.value(), descriptor.value());
+                    RETURN_IF_EXCEPTION(scope, false);
+                    if (!isSame)
+                        return typeError(exec, scope, throwException, ReadonlyPropertyChangeError);
+                }
             }
         }

trunk/Source/JavaScriptCore/runtime/PropertyDescriptor.cpp

@@ -1 +1 @@
 /*
- * Copyright (C) 2009, 2016 Apple Inc. All rights reserved.
+ * Copyright (C) 2009-2019 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -196 +196 @@
 bool PropertyDescriptor::equalTo(ExecState* exec, const PropertyDescriptor& other) const
 {
+    VM& vm = exec->vm();
+    auto scope = DECLARE_THROW_SCOPE(vm);
     if (other.m_value.isEmpty() != m_value.isEmpty()
         || other.m_getter.isEmpty() != m_getter.isEmpty()
         || other.m_setter.isEmpty() != m_setter.isEmpty())
         return false;
-    return (!m_value || sameValue(exec, other.m_value, m_value))
-        && (!m_getter || JSValue::strictEqual(exec, other.m_getter, m_getter))
+    if (m_value) {
+        bool isSame = sameValue(exec, other.m_value, m_value);
+        RETURN_IF_EXCEPTION(scope, false);
+        if (!isSame)
+            return false;
+    }
+    return (!m_getter || JSValue::strictEqual(exec, other.m_getter, m_getter))
         && (!m_setter || JSValue::strictEqual(exec, other.m_setter, m_setter))
         && attributesEqual(other);

trunk/Source/JavaScriptCore/runtime/ProxyObject.cpp

@@ -178 +178 @@
     if (result) {
         if (descriptor.isDataDescriptor() && !descriptor.configurable() && !descriptor.writable()) {
-            if (!sameValue(exec, descriptor.value(), trapResult))
+            bool isSame = sameValue(exec, descriptor.value(), trapResult);
+            RETURN_IF_EXCEPTION(scope, { });
+            if (!isSame)
                 return throwTypeError(exec, scope, "Proxy handler's 'get' result of a non-configurable and non-writable property should be the same value as the target's property"_s);
         } else if (descriptor.isAccessorDescriptor() && !descriptor.configurable() && descriptor.getter().isUndefined()) {
@@ -466 +468 @@
     if (hasProperty) {
         if (descriptor.isDataDescriptor() && !descriptor.configurable() && !descriptor.writable()) {
-            if (!sameValue(exec, descriptor.value(), putValue)) {
+            bool isSame = sameValue(exec, descriptor.value(), putValue);
+            RETURN_IF_EXCEPTION(scope, false);
+            if (!isSame) {
                 throwVMTypeError(exec, scope, "Proxy handler's 'set' on a non-configurable and non-writable property on 'target' should either return false or be the same value already on the 'target'"_s);
                 return false;
@@ -1148 +1152 @@
     JSValue targetPrototype = target->getPrototype(vm, exec);
     RETURN_IF_EXCEPTION(scope, false);
-    if (!sameValue(exec, prototype, targetPrototype)) {
+    bool isSame = sameValue(exec, prototype, targetPrototype);
+    RETURN_IF_EXCEPTION(scope, false);
+    if (!isSame) {
         throwVMTypeError(exec, scope, "Proxy 'setPrototypeOf' trap returned true when its target is non-extensible and the new prototype value is not the same as the current prototype value. It should have returned false"_s);
         return false;
@@ -1206 +1212 @@
     JSValue targetPrototype = target->getPrototype(vm, exec);
     RETURN_IF_EXCEPTION(scope, { });
-    if (!sameValue(exec, targetPrototype, trapResult)) {
+    bool isSame = sameValue(exec, targetPrototype, trapResult);
+    RETURN_IF_EXCEPTION(scope, { });
+    if (!isSame) {
         throwVMTypeError(exec, scope, "Proxy's 'getPrototypeOf' trap for a non-extensible target should return the same value as the target's prototype"_s);
         return { };

trunk/Source/JavaScriptCore/runtime/RegExpObject.cpp

@@ -1 +1 @@
 /*
  *  Copyright (C) 1999-2000 Harri Porten (porten@kde.org)
- *  Copyright (C) 2003-2018 Apple Inc. All Rights Reserved.
+ *  Copyright (C) 2003-2019 Apple Inc. All Rights Reserved.
  *
  *  This library is free software; you can redistribute it and/or
@@ -120 +120 @@
             if (descriptor.writablePresent() && descriptor.writable())
                 return typeError(exec, scope, shouldThrow, UnconfigurablePropertyChangeWritabilityError);
-            if (descriptor.value() && !sameValue(exec, regExp->getLastIndex(), descriptor.value()))
-                return typeError(exec, scope, shouldThrow, ReadonlyPropertyChangeError);
+            if (descriptor.value()) {
+                bool isSame = sameValue(exec, regExp->getLastIndex(), descriptor.value());
+                RETURN_IF_EXCEPTION(scope, false);
+                if (!isSame)
+                    return typeError(exec, scope, shouldThrow, ReadonlyPropertyChangeError);
+            }
             return true;
         }