Add missing checks after calls to the sameValue() JSValue comparator.
authormark.lam@apple.com <mark.lam@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 18 Oct 2019 04:35:28 +0000 (04:35 +0000)
committermark.lam@apple.com <mark.lam@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 18 Oct 2019 04:35:28 +0000 (04:35 +0000)
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):

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

JSTests/ChangeLog
JSTests/stress/validate-exception-check-in-proxy-object-put.js [new file with mode: 0644]
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/runtime/JSFunction.cpp
Source/JavaScriptCore/runtime/JSObject.cpp
Source/JavaScriptCore/runtime/PropertyDescriptor.cpp
Source/JavaScriptCore/runtime/ProxyObject.cpp
Source/JavaScriptCore/runtime/RegExpObject.cpp

index 59b0cc8..5273355 100644 (file)
@@ -1,3 +1,13 @@
+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>
 
         GetByVal and PutByVal on ArrayStorage need to use the same AbstractHeap
diff --git a/JSTests/stress/validate-exception-check-in-proxy-object-put.js b/JSTests/stress/validate-exception-check-in-proxy-object-put.js
new file mode 100644 (file)
index 0000000..d96e917
--- /dev/null
@@ -0,0 +1,13 @@
+const h = {
+  set: ()=>1,
+};
+const t = new String('b');
+const p = new Proxy(t, h);
+try {
+    p[0] = 'a' + 'a';
+} catch (e) {
+    exception = e;
+}
+
+if (exception != "TypeError: 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'")
+    throw "FAILED";
index d50a5d6..867de01 100644 (file)
@@ -1,3 +1,26 @@
+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>
 
         GetByVal and PutByVal on ArrayStorage need to use the same AbstractHeap
index eaaddd5..8603ccf 100644 (file)
@@ -591,12 +591,20 @@ bool JSFunction::defineOwnProperty(JSObject* object, ExecState* exec, PropertyNa
         if (!thisObject->jsExecutable()->hasCallerAndArgumentsProperties())
             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);
         RELEASE_AND_RETURN(scope, Base::defineOwnProperty(object, exec, propertyName, descriptor, throwException));
index 718c156..c324f72 100644 (file)
@@ -2683,8 +2683,12 @@ bool JSObject::defineOwnIndexedProperty(ExecState* exec, unsigned index, const P
                     return typeError(exec, scope, throwException, UnconfigurablePropertyChangeWritabilityError);
                 // 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.
         } else {
@@ -3641,8 +3645,12 @@ bool validateAndApplyPropertyDescriptor(ExecState* exec, JSObject* object, Prope
             if (!current.writable() && descriptor.writable())
                 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);
+                }
             }
         }
         if (current.attributesEqual(descriptor) && !descriptor.value())
index 1b4f5e6..4dcf567 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * 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
  * modification, are permitted provided that the following conditions
@@ -195,12 +195,19 @@ void PropertyDescriptor::setGetter(JSValue getter)
 
 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);
 }
index 8be8182..a2c7094 100644 (file)
@@ -177,7 +177,9 @@ static JSValue performProxyGet(ExecState* exec, ProxyObject* proxyObject, JSValu
     EXCEPTION_ASSERT(!scope.exception() || !result);
     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()) {
             if (!trapResult.isUndefined())
@@ -465,7 +467,9 @@ bool ProxyObject::performPut(ExecState* exec, JSValue putValue, JSValue thisValu
     EXCEPTION_ASSERT(!scope.exception() || !hasProperty);
     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;
             }
@@ -1147,7 +1151,9 @@ bool ProxyObject::performSetPrototype(ExecState* exec, JSValue prototype, bool s
 
     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;
     }
@@ -1205,7 +1211,9 @@ JSValue ProxyObject::performGetPrototype(ExecState* exec)
 
     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 { };
     }
index 3a9d9a7..69e1750 100644 (file)
@@ -1,6 +1,6 @@
 /*
  *  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
  *  modify it under the terms of the GNU Lesser General Public
@@ -119,8 +119,12 @@ bool RegExpObject::defineOwnProperty(JSObject* object, ExecState* exec, Property
         if (!regExp->lastIndexIsWritable()) {
             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;
         }
         if (descriptor.value()) {