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):
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@251274 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/JSTests/ChangeLog b/JSTests/ChangeLog
index 59b0cc8..5273355 100644
--- a/JSTests/ChangeLog
+++ b/JSTests/ChangeLog
@@ -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
index 0000000..d96e917
--- /dev/null
+++ b/JSTests/stress/validate-exception-check-in-proxy-object-put.js
@@ -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";
diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
index d50a5d6..867de01 100644
--- a/Source/JavaScriptCore/ChangeLog
+++ b/Source/JavaScriptCore/ChangeLog
@@ -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
diff --git a/Source/JavaScriptCore/runtime/JSFunction.cpp b/Source/JavaScriptCore/runtime/JSFunction.cpp
index eaaddd5..8603ccf 100644
--- a/Source/JavaScriptCore/runtime/JSFunction.cpp
+++ b/Source/JavaScriptCore/runtime/JSFunction.cpp
@@ -591,12 +591,20 @@
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));
diff --git a/Source/JavaScriptCore/runtime/JSObject.cpp b/Source/JavaScriptCore/runtime/JSObject.cpp
index 718c156..c324f72 100644
--- a/Source/JavaScriptCore/runtime/JSObject.cpp
+++ b/Source/JavaScriptCore/runtime/JSObject.cpp
@@ -2683,8 +2683,12 @@
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 @@
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())
diff --git a/Source/JavaScriptCore/runtime/PropertyDescriptor.cpp b/Source/JavaScriptCore/runtime/PropertyDescriptor.cpp
index 1b4f5e6c..4dcf567 100644
--- a/Source/JavaScriptCore/runtime/PropertyDescriptor.cpp
+++ b/Source/JavaScriptCore/runtime/PropertyDescriptor.cpp
@@ -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 @@
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);
}
diff --git a/Source/JavaScriptCore/runtime/ProxyObject.cpp b/Source/JavaScriptCore/runtime/ProxyObject.cpp
index 8be8182..a2c7094 100644
--- a/Source/JavaScriptCore/runtime/ProxyObject.cpp
+++ b/Source/JavaScriptCore/runtime/ProxyObject.cpp
@@ -177,7 +177,9 @@
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 @@
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 @@
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 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 { };
}
diff --git a/Source/JavaScriptCore/runtime/RegExpObject.cpp b/Source/JavaScriptCore/runtime/RegExpObject.cpp
index 3a9d9a7..69e1750 100644
--- a/Source/JavaScriptCore/runtime/RegExpObject.cpp
+++ b/Source/JavaScriptCore/runtime/RegExpObject.cpp
@@ -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 @@
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()) {
