<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[196721] trunk/Source/JavaScriptCore</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/196721">196721</a></dd>
<dt>Author</dt> <dd>mark.lam@apple.com</dd>
<dt>Date</dt> <dd>2016-02-17 14:03:56 -0800 (Wed, 17 Feb 2016)</dd>
</dl>
<h3>Log Message</h3>
<pre>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):</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceJavaScriptCoreChangeLog">trunk/Source/JavaScriptCore/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCoreruntimeStringPrototypecpp">trunk/Source/JavaScriptCore/runtime/StringPrototype.cpp</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ChangeLog (196720 => 196721)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog 2016-02-17 21:46:12 UTC (rev 196720)
+++ trunk/Source/JavaScriptCore/ChangeLog 2016-02-17 22:03:56 UTC (rev 196721)
</span><span class="lines">@@ -1,3 +1,45 @@
</span><ins>+2016-02-17 Mark Lam <mark.lam@apple.com>
+
+ 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):
+
</ins><span class="cx"> 2016-02-17 Commit Queue <commit-queue@webkit.org>
</span><span class="cx">
</span><span class="cx"> Unreviewed, rolling out r196675.
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreruntimeStringPrototypecpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/runtime/StringPrototype.cpp (196720 => 196721)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/runtime/StringPrototype.cpp 2016-02-17 21:46:12 UTC (rev 196720)
+++ trunk/Source/JavaScriptCore/runtime/StringPrototype.cpp 2016-02-17 22:03:56 UTC (rev 196721)
</span><span class="lines">@@ -490,13 +490,16 @@
</span><span class="cx"> String replacementString;
</span><span class="cx"> CallData callData;
</span><span class="cx"> CallType callType = getCallData(replaceValue, callData);
</span><del>- if (callType == CallTypeNone)
</del><ins>+ if (callType == CallTypeNone) {
</ins><span class="cx"> replacementString = replaceValue.toString(exec)->value(exec);
</span><ins>+ if (exec->hadException())
+ return JSValue::encode(jsUndefined());
+ }
</ins><span class="cx">
</span><span class="cx"> const String& source = string->value(exec);
</span><span class="cx"> unsigned sourceLen = source.length();
</span><span class="cx"> if (exec->hadException())
</span><del>- return JSValue::encode(JSValue());
</del><ins>+ return JSValue::encode(jsUndefined());
</ins><span class="cx"> RegExpObject* regExpObject = asRegExpObject(searchValue);
</span><span class="cx"> RegExp* regExp = regExpObject->regExp();
</span><span class="cx"> bool global = regExp->global();
</span><span class="lines">@@ -505,7 +508,7 @@
</span><span class="cx"> // ES5.1 15.5.4.10 step 8.a.
</span><span class="cx"> regExpObject->setLastIndex(exec, 0);
</span><span class="cx"> if (exec->hadException())
</span><del>- return JSValue::encode(JSValue());
</del><ins>+ return JSValue::encode(jsUndefined());
</ins><span class="cx">
</span><span class="cx"> if (callType == CallTypeNone && !replacementString.length())
</span><span class="cx"> return removeUsingRegExpSearch(exec, string, source, regExp);
</span><span class="lines">@@ -526,7 +529,7 @@
</span><span class="cx"> JSFunction* func = jsCast<JSFunction*>(replaceValue);
</span><span class="cx"> CachedCall cachedCall(exec, func, argCount);
</span><span class="cx"> if (exec->hadException())
</span><del>- return JSValue::encode(jsNull());
</del><ins>+ return JSValue::encode(jsUndefined());
</ins><span class="cx"> VM* vm = &exec->vm();
</span><span class="cx"> if (source.is8Bit()) {
</span><span class="cx"> while (true) {
</span><span class="lines">@@ -555,7 +558,7 @@
</span><span class="cx"> JSValue jsResult = cachedCall.call();
</span><span class="cx"> replacements.append(jsResult.toString(exec)->value(exec));
</span><span class="cx"> if (exec->hadException())
</span><del>- break;
</del><ins>+ return JSValue::encode(jsUndefined());
</ins><span class="cx">
</span><span class="cx"> lastIndex = result.end;
</span><span class="cx"> startPosition = lastIndex;
</span><span class="lines">@@ -594,7 +597,7 @@
</span><span class="cx"> JSValue jsResult = cachedCall.call();
</span><span class="cx"> replacements.append(jsResult.toString(exec)->value(exec));
</span><span class="cx"> if (exec->hadException())
</span><del>- break;
</del><ins>+ return JSValue::encode(jsUndefined());
</ins><span class="cx">
</span><span class="cx"> lastIndex = result.end;
</span><span class="cx"> startPosition = lastIndex;
</span><span class="lines">@@ -635,7 +638,7 @@
</span><span class="cx">
</span><span class="cx"> replacements.append(call(exec, replaceValue, callType, callData, jsUndefined(), args).toString(exec)->value(exec));
</span><span class="cx"> if (exec->hadException())
</span><del>- break;
</del><ins>+ return JSValue::encode(jsUndefined());
</ins><span class="cx"> } else {
</span><span class="cx"> int replLen = replacementString.length();
</span><span class="cx"> if (lastIndex < result.start || replLen) {
</span><span class="lines">@@ -990,7 +993,7 @@
</span><span class="cx"> // ES5.1 15.5.4.10 step 8.a.
</span><span class="cx"> regExpObject->setLastIndex(exec, 0);
</span><span class="cx"> if (exec->hadException())
</span><del>- return JSValue::encode(JSValue());
</del><ins>+ return JSValue::encode(jsUndefined());
</ins><span class="cx"> }
</span><span class="cx"> } else {
</span><span class="cx"> /*
</span><span class="lines">@@ -999,7 +1002,13 @@
</span><span class="cx"> * replaced with the result of the expression new RegExp(regexp).
</span><span class="cx"> * Per ECMA 15.10.4.1, if a0 is undefined substitute the empty string.
</span><span class="cx"> */
</span><del>- regExp = RegExp::create(exec->vm(), a0.isUndefined() ? emptyString() : a0.toString(exec)->value(exec), NoFlags);
</del><ins>+ 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);
</ins><span class="cx"> if (!regExp->isValid())
</span><span class="cx"> return throwVMError(exec, createSyntaxError(exec, regExp->errorMessage()));
</span><span class="cx"> }
</span><span class="lines">@@ -1042,6 +1051,9 @@
</span><span class="cx"> if (!checkObjectCoercible(thisValue))
</span><span class="cx"> return throwVMTypeError(exec);
</span><span class="cx"> String s = thisValue.toString(exec)->value(exec);
</span><ins>+ if (exec->hadException())
+ return JSValue::encode(jsUndefined());
+
</ins><span class="cx"> int len = s.length();
</span><span class="cx"> RELEASE_ASSERT(len >= 0);
</span><span class="cx">
</span><span class="lines">@@ -1104,6 +1116,9 @@
</span><span class="cx"> // 2. Let S be the result of calling ToString, giving it the this value as its argument.
</span><span class="cx"> // 6. Let s be the number of characters in S.
</span><span class="cx"> String input = thisValue.toString(exec)->value(exec);
</span><ins>+ if (exec->hadException())
+ return JSValue::encode(jsUndefined());
+ ASSERT(!input.isNull());
</ins><span class="cx">
</span><span class="cx"> // 3. Let A be a new array created as if by the expression new Array()
</span><span class="cx"> // where Array is the standard built-in constructor with that name.
</span><span class="lines">@@ -1230,6 +1245,8 @@
</span><span class="cx"> }
</span><span class="cx"> } else {
</span><span class="cx"> String separator = separatorValue.toString(exec)->value(exec);
</span><ins>+ if (exec->hadException())
+ return JSValue::encode(jsUndefined());
</ins><span class="cx">
</span><span class="cx"> // 9. If lim == 0, return A.
</span><span class="cx"> if (!limit)
</span><span class="lines">@@ -1439,9 +1456,14 @@
</span><span class="cx"> if (!checkObjectCoercible(thisValue))
</span><span class="cx"> return throwVMTypeError(exec);
</span><span class="cx"> String s = thisValue.toString(exec)->value(exec);
</span><ins>+ if (exec->hadException())
+ return JSValue::encode(jsUndefined());
</ins><span class="cx">
</span><span class="cx"> JSValue a0 = exec->argument(0);
</span><del>- return JSValue::encode(jsNumber(Collator().collate(s, a0.toString(exec)->value(exec))));
</del><ins>+ String str = a0.toString(exec)->value(exec);
+ if (exec->hadException())
+ return JSValue::encode(jsUndefined());
+ return JSValue::encode(jsNumber(Collator().collate(s, str)));
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> #if ENABLE(INTL)
</span><span class="lines">@@ -1549,6 +1571,8 @@
</span><span class="cx"> if (!checkObjectCoercible(thisValue))
</span><span class="cx"> return throwVMTypeError(exec);
</span><span class="cx"> String s = thisValue.toString(exec)->value(exec);
</span><ins>+ if (exec->hadException())
+ return JSValue::encode(jsUndefined());
</ins><span class="cx"> return JSValue::encode(jsMakeNontrivialString(exec, "<big>", s, "</big>"));
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -1558,6 +1582,8 @@
</span><span class="cx"> if (!checkObjectCoercible(thisValue))
</span><span class="cx"> return throwVMTypeError(exec);
</span><span class="cx"> String s = thisValue.toString(exec)->value(exec);
</span><ins>+ if (exec->hadException())
+ return JSValue::encode(jsUndefined());
</ins><span class="cx"> return JSValue::encode(jsMakeNontrivialString(exec, "<small>", s, "</small>"));
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -1567,6 +1593,8 @@
</span><span class="cx"> if (!checkObjectCoercible(thisValue))
</span><span class="cx"> return throwVMTypeError(exec);
</span><span class="cx"> String s = thisValue.toString(exec)->value(exec);
</span><ins>+ if (exec->hadException())
+ return JSValue::encode(jsUndefined());
</ins><span class="cx"> return JSValue::encode(jsMakeNontrivialString(exec, "<blink>", s, "</blink>"));
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -1576,6 +1604,8 @@
</span><span class="cx"> if (!checkObjectCoercible(thisValue))
</span><span class="cx"> return throwVMTypeError(exec);
</span><span class="cx"> String s = thisValue.toString(exec)->value(exec);
</span><ins>+ if (exec->hadException())
+ return JSValue::encode(jsUndefined());
</ins><span class="cx"> return JSValue::encode(jsMakeNontrivialString(exec, "<b>", s, "</b>"));
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -1585,6 +1615,8 @@
</span><span class="cx"> if (!checkObjectCoercible(thisValue))
</span><span class="cx"> return throwVMTypeError(exec);
</span><span class="cx"> String s = thisValue.toString(exec)->value(exec);
</span><ins>+ if (exec->hadException())
+ return JSValue::encode(jsUndefined());
</ins><span class="cx"> return JSValue::encode(jsMakeNontrivialString(exec, "<tt>", s, "</tt>"));
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -1594,6 +1626,8 @@
</span><span class="cx"> if (!checkObjectCoercible(thisValue))
</span><span class="cx"> return throwVMTypeError(exec);
</span><span class="cx"> String s = thisValue.toString(exec)->value(exec);
</span><ins>+ if (exec->hadException())
+ return JSValue::encode(jsUndefined());
</ins><span class="cx"> return JSValue::encode(jsMakeNontrivialString(exec, "<i>", s, "</i>"));
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -1603,6 +1637,8 @@
</span><span class="cx"> if (!checkObjectCoercible(thisValue))
</span><span class="cx"> return throwVMTypeError(exec);
</span><span class="cx"> String s = thisValue.toString(exec)->value(exec);
</span><ins>+ if (exec->hadException())
+ return JSValue::encode(jsUndefined());
</ins><span class="cx"> return JSValue::encode(jsMakeNontrivialString(exec, "<strike>", s, "</strike>"));
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -1612,6 +1648,8 @@
</span><span class="cx"> if (!checkObjectCoercible(thisValue))
</span><span class="cx"> return throwVMTypeError(exec);
</span><span class="cx"> String s = thisValue.toString(exec)->value(exec);
</span><ins>+ if (exec->hadException())
+ return JSValue::encode(jsUndefined());
</ins><span class="cx"> return JSValue::encode(jsMakeNontrivialString(exec, "<sub>", s, "</sub>"));
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -1621,6 +1659,8 @@
</span><span class="cx"> if (!checkObjectCoercible(thisValue))
</span><span class="cx"> return throwVMTypeError(exec);
</span><span class="cx"> String s = thisValue.toString(exec)->value(exec);
</span><ins>+ if (exec->hadException())
+ return JSValue::encode(jsUndefined());
</ins><span class="cx"> return JSValue::encode(jsMakeNontrivialString(exec, "<sup>", s, "</sup>"));
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -1630,6 +1670,9 @@
</span><span class="cx"> if (!checkObjectCoercible(thisValue))
</span><span class="cx"> return throwVMTypeError(exec);
</span><span class="cx"> String s = thisValue.toString(exec)->value(exec);
</span><ins>+ if (exec->hadException())
+ return JSValue::encode(jsUndefined());
+
</ins><span class="cx"> JSValue a0 = exec->argument(0);
</span><span class="cx"> String color = a0.toWTFString(exec);
</span><span class="cx"> color.replaceWithLiteral('"', "&quot;");
</span><span class="lines">@@ -1643,6 +1686,9 @@
</span><span class="cx"> if (!checkObjectCoercible(thisValue))
</span><span class="cx"> return throwVMTypeError(exec);
</span><span class="cx"> String s = thisValue.toString(exec)->value(exec);
</span><ins>+ if (exec->hadException())
+ return JSValue::encode(jsUndefined());
+
</ins><span class="cx"> JSValue a0 = exec->argument(0);
</span><span class="cx">
</span><span class="cx"> uint32_t smallInteger;
</span><span class="lines">@@ -1692,6 +1738,9 @@
</span><span class="cx"> if (!checkObjectCoercible(thisValue))
</span><span class="cx"> return throwVMTypeError(exec);
</span><span class="cx"> String s = thisValue.toString(exec)->value(exec);
</span><ins>+ if (exec->hadException())
+ return JSValue::encode(jsUndefined());
+
</ins><span class="cx"> JSValue a0 = exec->argument(0);
</span><span class="cx"> String anchor = a0.toWTFString(exec);
</span><span class="cx"> anchor.replaceWithLiteral('"', "&quot;");
</span><span class="lines">@@ -1705,6 +1754,9 @@
</span><span class="cx"> if (!checkObjectCoercible(thisValue))
</span><span class="cx"> return throwVMTypeError(exec);
</span><span class="cx"> String s = thisValue.toString(exec)->value(exec);
</span><ins>+ if (exec->hadException())
+ return JSValue::encode(jsUndefined());
+
</ins><span class="cx"> JSValue a0 = exec->argument(0);
</span><span class="cx"> String linkText = a0.toWTFString(exec);
</span><span class="cx"> linkText.replaceWithLiteral('"', "&quot;");
</span><span class="lines">@@ -1747,6 +1799,9 @@
</span><span class="cx"> if (!checkObjectCoercible(thisValue))
</span><span class="cx"> return throwTypeError(exec);
</span><span class="cx"> String str = thisValue.toString(exec)->value(exec);
</span><ins>+ if (exec->hadException())
+ return jsUndefined();
+
</ins><span class="cx"> unsigned left = 0;
</span><span class="cx"> if (trimKind & TrimLeft) {
</span><span class="cx"> while (left < str.length() && isStrWhiteSpace(str[left]))
</span></span></pre>
</div>
</div>
</body>
</html>