Callers of JSString::getIndex should check for OOM exceptions
authorkeith_miller@apple.com <keith_miller@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 14 Dec 2018 20:53:08 +0000 (20:53 +0000)
committerkeith_miller@apple.com <keith_miller@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 14 Dec 2018 20:53:08 +0000 (20:53 +0000)
https://bugs.webkit.org/show_bug.cgi?id=192709

Reviewed by Mark Lam.

JSTests:

* stress/StringObject-define-length-getter-rope-string-oom.js: Added.

Source/JavaScriptCore:

This patch also allows Strings to OOM when the StringObject wrapper
attempts to look up an own property on the string.

Remove isExtensibleImpl because it's only used in one place and call
isStructureExtensible instead.

* runtime/JSObject.cpp:
(JSC::JSObject::isExtensible):
* runtime/JSObject.h:
(JSC::JSObject::isExtensibleImpl): Deleted.
* runtime/JSString.h:
(JSC::JSString::getStringPropertySlot):
* runtime/StringObject.cpp:
(JSC::StringObject::defineOwnProperty):

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

JSTests/ChangeLog
JSTests/stress/StringObject-define-length-getter-rope-string-oom.js [new file with mode: 0644]
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/runtime/JSObject.cpp
Source/JavaScriptCore/runtime/JSObject.h
Source/JavaScriptCore/runtime/JSString.h
Source/JavaScriptCore/runtime/StringObject.cpp

index 06b6f33..d82e421 100644 (file)
@@ -1,3 +1,12 @@
+2018-12-14  Keith Miller  <keith_miller@apple.com>
+
+        Callers of JSString::getIndex should check for OOM exceptions
+        https://bugs.webkit.org/show_bug.cgi?id=192709
+
+        Reviewed by Mark Lam.
+
+        * stress/StringObject-define-length-getter-rope-string-oom.js: Added.
+
 2018-12-13  Mark Lam  <mark.lam@apple.com>
 
         Add a missing exception check.
diff --git a/JSTests/stress/StringObject-define-length-getter-rope-string-oom.js b/JSTests/stress/StringObject-define-length-getter-rope-string-oom.js
new file mode 100644 (file)
index 0000000..e7e706c
--- /dev/null
@@ -0,0 +1,5 @@
+try {
+    let char16 = decodeURI('%E7%9A%84');
+    let rope = char16.padEnd(2147483644, 1);
+    rope.__defineGetter__(256, function () {});
+} catch { }
index 9bd2412..991b84d 100644 (file)
@@ -1,3 +1,25 @@
+2018-12-14  Keith Miller  <keith_miller@apple.com>
+
+        Callers of JSString::getIndex should check for OOM exceptions
+        https://bugs.webkit.org/show_bug.cgi?id=192709
+
+        Reviewed by Mark Lam.
+
+        This patch also allows Strings to OOM when the StringObject wrapper
+        attempts to look up an own property on the string.
+
+        Remove isExtensibleImpl because it's only used in one place and call
+        isStructureExtensible instead.
+
+        * runtime/JSObject.cpp:
+        (JSC::JSObject::isExtensible):
+        * runtime/JSObject.h:
+        (JSC::JSObject::isExtensibleImpl): Deleted.
+        * runtime/JSString.h:
+        (JSC::JSString::getStringPropertySlot):
+        * runtime/StringObject.cpp:
+        (JSC::StringObject::defineOwnProperty):
+
 2018-12-13  Fujii Hironori  <Hironori.Fujii@sony.com>
 
         [WinCairo][Clang] DLLLauncherMain.cpp: warning: unused function 'prependPath' and 'appleApplicationSupportDirectory'
index b7fa795..20fcd40 100644 (file)
@@ -2431,7 +2431,7 @@ bool JSObject::preventExtensions(JSObject* object, ExecState* exec)
 
 bool JSObject::isExtensible(JSObject* obj, ExecState* exec)
 {
-    return obj->isExtensibleImpl(exec->vm());
+    return obj->isStructureExtensible(exec->vm());
 }
 
 bool JSObject::isExtensible(ExecState* exec)
index 148b1b1..a634ae1 100644 (file)
@@ -753,7 +753,6 @@ public:
 
 private:
     NonPropertyTransition suggestedArrayStorageTransition(VM&) const;
-    ALWAYS_INLINE bool isExtensibleImpl(VM& vm) { return isStructureExtensible(vm); }
 public:
     // You should only call isStructureExtensible() when:
     // - Performing this check in a way that isn't described in the specification 
index 605f40a..e0c04de 100644 (file)
@@ -687,6 +687,8 @@ ALWAYS_INLINE JSString* jsStringWithCache(ExecState* exec, const String& s)
 ALWAYS_INLINE bool JSString::getStringPropertySlot(ExecState* exec, PropertyName propertyName, PropertySlot& slot)
 {
     VM& vm = exec->vm();
+    auto scope = DECLARE_THROW_SCOPE(vm);
+
     if (propertyName == vm.propertyNames->length) {
         slot.setValue(this, PropertyAttribute::DontEnum | PropertyAttribute::DontDelete | PropertyAttribute::ReadOnly, jsNumber(length()));
         return true;
@@ -694,7 +696,9 @@ ALWAYS_INLINE bool JSString::getStringPropertySlot(ExecState* exec, PropertyName
 
     std::optional<uint32_t> index = parseIndex(propertyName);
     if (index && index.value() < length()) {
-        slot.setValue(this, PropertyAttribute::DontDelete | PropertyAttribute::ReadOnly, getIndex(exec, index.value()));
+        JSValue value = getIndex(exec, index.value());
+        RETURN_IF_EXCEPTION(scope, false);
+        slot.setValue(this, PropertyAttribute::DontDelete | PropertyAttribute::ReadOnly, value);
         return true;
     }
 
@@ -703,8 +707,13 @@ ALWAYS_INLINE bool JSString::getStringPropertySlot(ExecState* exec, PropertyName
 
 ALWAYS_INLINE bool JSString::getStringPropertySlot(ExecState* exec, unsigned propertyName, PropertySlot& slot)
 {
+    VM& vm = exec->vm();
+    auto scope = DECLARE_THROW_SCOPE(vm);
+
     if (propertyName < length()) {
-        slot.setValue(this, PropertyAttribute::DontDelete | PropertyAttribute::ReadOnly, getIndex(exec, propertyName));
+        JSValue value = getIndex(exec, propertyName);
+        RETURN_IF_EXCEPTION(scope, false);
+        slot.setValue(this, PropertyAttribute::DontDelete | PropertyAttribute::ReadOnly, value);
         return true;
     }
 
index 504a600..234ef89 100644 (file)
@@ -114,7 +114,8 @@ bool StringObject::defineOwnProperty(JSObject* object, ExecState* exec, Property
         // https://tc39.github.io/ecma262/#sec-string-exotic-objects-getownproperty-p
         PropertyDescriptor current;
         bool isCurrentDefined = thisObject->getOwnPropertyDescriptor(exec, propertyName, current);
-        ASSERT(isCurrentDefined);
+        EXCEPTION_ASSERT(!scope.exception() == isCurrentDefined);
+        RETURN_IF_EXCEPTION(scope, false);
         bool isExtensible = thisObject->isExtensible(exec);
         RETURN_IF_EXCEPTION(scope, false);
         RELEASE_AND_RETURN(scope, validateAndApplyPropertyDescriptor(exec, nullptr, propertyName, isExtensible, descriptor, isCurrentDefined, current, throwException));