Title: [196721] trunk/Source/_javascript_Core
Revision
196721
Author
[email protected]
Date
2016-02-17 14:03:56 -0800 (Wed, 17 Feb 2016)

Log Message

StringPrototype functions should check for exceptions after calling JSString::value().
https://bugs.webkit.org/show_bug.cgi?id=154340

Reviewed by Filip Pizlo.

JSString::value() can throw an exception if the JS string is a rope and value()
needs to resolve the rope but encounters an OutOfMemory error.  If value() is not
able to resolve the rope, it will return a null string (in addition to throwing
the exception).  If StringPrototype functions do not check for exceptions after
calling JSString::value(), they may eventually use the returned null string and
crash the VM.

The fix is to add all the necessary exception checks, and do the appropriate
handling if needed.

Also in a few place where when an exception is detected, we return JSValue(), I
changed it to return jsUndefined() instead to be consistent with the rest of the
file.

* runtime/StringPrototype.cpp:
(JSC::replaceUsingRegExpSearch):
(JSC::stringProtoFuncMatch):
(JSC::stringProtoFuncSlice):
(JSC::stringProtoFuncSplit):
(JSC::stringProtoFuncLocaleCompare):
(JSC::stringProtoFuncBig):
(JSC::stringProtoFuncSmall):
(JSC::stringProtoFuncBlink):
(JSC::stringProtoFuncBold):
(JSC::stringProtoFuncFixed):
(JSC::stringProtoFuncItalics):
(JSC::stringProtoFuncStrike):
(JSC::stringProtoFuncSub):
(JSC::stringProtoFuncSup):
(JSC::stringProtoFuncFontcolor):
(JSC::stringProtoFuncFontsize):
(JSC::stringProtoFuncAnchor):
(JSC::stringProtoFuncLink):
(JSC::trimString):

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (196720 => 196721)


--- trunk/Source/_javascript_Core/ChangeLog	2016-02-17 21:46:12 UTC (rev 196720)
+++ trunk/Source/_javascript_Core/ChangeLog	2016-02-17 22:03:56 UTC (rev 196721)
@@ -1,3 +1,45 @@
+2016-02-17  Mark Lam  <[email protected]>
+
+        StringPrototype functions should check for exceptions after calling JSString::value().
+        https://bugs.webkit.org/show_bug.cgi?id=154340
+
+        Reviewed by Filip Pizlo.
+
+        JSString::value() can throw an exception if the JS string is a rope and value()
+        needs to resolve the rope but encounters an OutOfMemory error.  If value() is not
+        able to resolve the rope, it will return a null string (in addition to throwing
+        the exception).  If StringPrototype functions do not check for exceptions after
+        calling JSString::value(), they may eventually use the returned null string and
+        crash the VM.
+
+        The fix is to add all the necessary exception checks, and do the appropriate
+        handling if needed.
+
+        Also in a few place where when an exception is detected, we return JSValue(), I
+        changed it to return jsUndefined() instead to be consistent with the rest of the
+        file.
+
+        * runtime/StringPrototype.cpp:
+        (JSC::replaceUsingRegExpSearch):
+        (JSC::stringProtoFuncMatch):
+        (JSC::stringProtoFuncSlice):
+        (JSC::stringProtoFuncSplit):
+        (JSC::stringProtoFuncLocaleCompare):
+        (JSC::stringProtoFuncBig):
+        (JSC::stringProtoFuncSmall):
+        (JSC::stringProtoFuncBlink):
+        (JSC::stringProtoFuncBold):
+        (JSC::stringProtoFuncFixed):
+        (JSC::stringProtoFuncItalics):
+        (JSC::stringProtoFuncStrike):
+        (JSC::stringProtoFuncSub):
+        (JSC::stringProtoFuncSup):
+        (JSC::stringProtoFuncFontcolor):
+        (JSC::stringProtoFuncFontsize):
+        (JSC::stringProtoFuncAnchor):
+        (JSC::stringProtoFuncLink):
+        (JSC::trimString):
+
 2016-02-17  Commit Queue  <[email protected]>
 
         Unreviewed, rolling out r196675.

Modified: trunk/Source/_javascript_Core/runtime/StringPrototype.cpp (196720 => 196721)


--- trunk/Source/_javascript_Core/runtime/StringPrototype.cpp	2016-02-17 21:46:12 UTC (rev 196720)
+++ trunk/Source/_javascript_Core/runtime/StringPrototype.cpp	2016-02-17 22:03:56 UTC (rev 196721)
@@ -490,13 +490,16 @@
     String replacementString;
     CallData callData;
     CallType callType = getCallData(replaceValue, callData);
-    if (callType == CallTypeNone)
+    if (callType == CallTypeNone) {
         replacementString = replaceValue.toString(exec)->value(exec);
+        if (exec->hadException())
+            return JSValue::encode(jsUndefined());
+    }
 
     const String& source = string->value(exec);
     unsigned sourceLen = source.length();
     if (exec->hadException())
-        return JSValue::encode(JSValue());
+        return JSValue::encode(jsUndefined());
     RegExpObject* regExpObject = asRegExpObject(searchValue);
     RegExp* regExp = regExpObject->regExp();
     bool global = regExp->global();
@@ -505,7 +508,7 @@
         // ES5.1 15.5.4.10 step 8.a.
         regExpObject->setLastIndex(exec, 0);
         if (exec->hadException())
-            return JSValue::encode(JSValue());
+            return JSValue::encode(jsUndefined());
 
         if (callType == CallTypeNone && !replacementString.length())
             return removeUsingRegExpSearch(exec, string, source, regExp);
@@ -526,7 +529,7 @@
         JSFunction* func = jsCast<JSFunction*>(replaceValue);
         CachedCall cachedCall(exec, func, argCount);
         if (exec->hadException())
-            return JSValue::encode(jsNull());
+            return JSValue::encode(jsUndefined());
         VM* vm = &exec->vm();
         if (source.is8Bit()) {
             while (true) {
@@ -555,7 +558,7 @@
                 JSValue jsResult = cachedCall.call();
                 replacements.append(jsResult.toString(exec)->value(exec));
                 if (exec->hadException())
-                    break;
+                    return JSValue::encode(jsUndefined());
 
                 lastIndex = result.end;
                 startPosition = lastIndex;
@@ -594,7 +597,7 @@
                 JSValue jsResult = cachedCall.call();
                 replacements.append(jsResult.toString(exec)->value(exec));
                 if (exec->hadException())
-                    break;
+                    return JSValue::encode(jsUndefined());
 
                 lastIndex = result.end;
                 startPosition = lastIndex;
@@ -635,7 +638,7 @@
 
                 replacements.append(call(exec, replaceValue, callType, callData, jsUndefined(), args).toString(exec)->value(exec));
                 if (exec->hadException())
-                    break;
+                    return JSValue::encode(jsUndefined());
             } else {
                 int replLen = replacementString.length();
                 if (lastIndex < result.start || replLen) {
@@ -990,7 +993,7 @@
             // ES5.1 15.5.4.10 step 8.a.
             regExpObject->setLastIndex(exec, 0);
             if (exec->hadException())
-                return JSValue::encode(JSValue());
+                return JSValue::encode(jsUndefined());
         }
     } else {
         /*
@@ -999,7 +1002,13 @@
          *  replaced with the result of the _expression_ new RegExp(regexp).
          *  Per ECMA 15.10.4.1, if a0 is undefined substitute the empty string.
          */
-        regExp = RegExp::create(exec->vm(), a0.isUndefined() ? emptyString() : a0.toString(exec)->value(exec), NoFlags);
+        String patternString = emptyString();
+        if (!a0.isUndefined()) {
+            patternString = a0.toString(exec)->value(exec);
+            if (exec->hadException())
+                return JSValue::encode(jsUndefined());
+        }
+        regExp = RegExp::create(exec->vm(), patternString, NoFlags);
         if (!regExp->isValid())
             return throwVMError(exec, createSyntaxError(exec, regExp->errorMessage()));
     }
@@ -1042,6 +1051,9 @@
     if (!checkObjectCoercible(thisValue))
         return throwVMTypeError(exec);
     String s = thisValue.toString(exec)->value(exec);
+    if (exec->hadException())
+        return JSValue::encode(jsUndefined());
+
     int len = s.length();
     RELEASE_ASSERT(len >= 0);
 
@@ -1104,6 +1116,9 @@
     // 2. Let S be the result of calling ToString, giving it the this value as its argument.
     // 6. Let s be the number of characters in S.
     String input = thisValue.toString(exec)->value(exec);
+    if (exec->hadException())
+        return JSValue::encode(jsUndefined());
+    ASSERT(!input.isNull());
 
     // 3. Let A be a new array created as if by the _expression_ new Array()
     //    where Array is the standard built-in constructor with that name.
@@ -1230,6 +1245,8 @@
         }
     } else {
         String separator = separatorValue.toString(exec)->value(exec);
+        if (exec->hadException())
+            return JSValue::encode(jsUndefined());
 
         // 9. If lim == 0, return A.
         if (!limit)
@@ -1439,9 +1456,14 @@
     if (!checkObjectCoercible(thisValue))
         return throwVMTypeError(exec);
     String s = thisValue.toString(exec)->value(exec);
+    if (exec->hadException())
+        return JSValue::encode(jsUndefined());
 
     JSValue a0 = exec->argument(0);
-    return JSValue::encode(jsNumber(Collator().collate(s, a0.toString(exec)->value(exec))));
+    String str = a0.toString(exec)->value(exec);
+    if (exec->hadException())
+        return JSValue::encode(jsUndefined());
+    return JSValue::encode(jsNumber(Collator().collate(s, str)));
 }
 
 #if ENABLE(INTL)
@@ -1549,6 +1571,8 @@
     if (!checkObjectCoercible(thisValue))
         return throwVMTypeError(exec);
     String s = thisValue.toString(exec)->value(exec);
+    if (exec->hadException())
+        return JSValue::encode(jsUndefined());
     return JSValue::encode(jsMakeNontrivialString(exec, "<big>", s, "</big>"));
 }
 
@@ -1558,6 +1582,8 @@
     if (!checkObjectCoercible(thisValue))
         return throwVMTypeError(exec);
     String s = thisValue.toString(exec)->value(exec);
+    if (exec->hadException())
+        return JSValue::encode(jsUndefined());
     return JSValue::encode(jsMakeNontrivialString(exec, "<small>", s, "</small>"));
 }
 
@@ -1567,6 +1593,8 @@
     if (!checkObjectCoercible(thisValue))
         return throwVMTypeError(exec);
     String s = thisValue.toString(exec)->value(exec);
+    if (exec->hadException())
+        return JSValue::encode(jsUndefined());
     return JSValue::encode(jsMakeNontrivialString(exec, "<blink>", s, "</blink>"));
 }
 
@@ -1576,6 +1604,8 @@
     if (!checkObjectCoercible(thisValue))
         return throwVMTypeError(exec);
     String s = thisValue.toString(exec)->value(exec);
+    if (exec->hadException())
+        return JSValue::encode(jsUndefined());
     return JSValue::encode(jsMakeNontrivialString(exec, "<b>", s, "</b>"));
 }
 
@@ -1585,6 +1615,8 @@
     if (!checkObjectCoercible(thisValue))
         return throwVMTypeError(exec);
     String s = thisValue.toString(exec)->value(exec);
+    if (exec->hadException())
+        return JSValue::encode(jsUndefined());
     return JSValue::encode(jsMakeNontrivialString(exec, "<tt>", s, "</tt>"));
 }
 
@@ -1594,6 +1626,8 @@
     if (!checkObjectCoercible(thisValue))
         return throwVMTypeError(exec);
     String s = thisValue.toString(exec)->value(exec);
+    if (exec->hadException())
+        return JSValue::encode(jsUndefined());
     return JSValue::encode(jsMakeNontrivialString(exec, "<i>", s, "</i>"));
 }
 
@@ -1603,6 +1637,8 @@
     if (!checkObjectCoercible(thisValue))
         return throwVMTypeError(exec);
     String s = thisValue.toString(exec)->value(exec);
+    if (exec->hadException())
+        return JSValue::encode(jsUndefined());
     return JSValue::encode(jsMakeNontrivialString(exec, "<strike>", s, "</strike>"));
 }
 
@@ -1612,6 +1648,8 @@
     if (!checkObjectCoercible(thisValue))
         return throwVMTypeError(exec);
     String s = thisValue.toString(exec)->value(exec);
+    if (exec->hadException())
+        return JSValue::encode(jsUndefined());
     return JSValue::encode(jsMakeNontrivialString(exec, "<sub>", s, "</sub>"));
 }
 
@@ -1621,6 +1659,8 @@
     if (!checkObjectCoercible(thisValue))
         return throwVMTypeError(exec);
     String s = thisValue.toString(exec)->value(exec);
+    if (exec->hadException())
+        return JSValue::encode(jsUndefined());
     return JSValue::encode(jsMakeNontrivialString(exec, "<sup>", s, "</sup>"));
 }
 
@@ -1630,6 +1670,9 @@
     if (!checkObjectCoercible(thisValue))
         return throwVMTypeError(exec);
     String s = thisValue.toString(exec)->value(exec);
+    if (exec->hadException())
+        return JSValue::encode(jsUndefined());
+
     JSValue a0 = exec->argument(0);
     String color = a0.toWTFString(exec);
     color.replaceWithLiteral('"', "&quot;");
@@ -1643,6 +1686,9 @@
     if (!checkObjectCoercible(thisValue))
         return throwVMTypeError(exec);
     String s = thisValue.toString(exec)->value(exec);
+    if (exec->hadException())
+        return JSValue::encode(jsUndefined());
+
     JSValue a0 = exec->argument(0);
 
     uint32_t smallInteger;
@@ -1692,6 +1738,9 @@
     if (!checkObjectCoercible(thisValue))
         return throwVMTypeError(exec);
     String s = thisValue.toString(exec)->value(exec);
+    if (exec->hadException())
+        return JSValue::encode(jsUndefined());
+
     JSValue a0 = exec->argument(0);
     String anchor = a0.toWTFString(exec);
     anchor.replaceWithLiteral('"', "&quot;");
@@ -1705,6 +1754,9 @@
     if (!checkObjectCoercible(thisValue))
         return throwVMTypeError(exec);
     String s = thisValue.toString(exec)->value(exec);
+    if (exec->hadException())
+        return JSValue::encode(jsUndefined());
+
     JSValue a0 = exec->argument(0);
     String linkText = a0.toWTFString(exec);
     linkText.replaceWithLiteral('"', "&quot;");
@@ -1747,6 +1799,9 @@
     if (!checkObjectCoercible(thisValue))
         return throwTypeError(exec);
     String str = thisValue.toString(exec)->value(exec);
+    if (exec->hadException())
+        return jsUndefined();
+
     unsigned left = 0;
     if (trimKind & TrimLeft) {
         while (left < str.length() && isStrWhiteSpace(str[left]))
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to