- Revision
- 196745
- Author
- [email protected]
- Date
- 2016-02-17 22:28:26 -0800 (Wed, 17 Feb 2016)
Log Message
Callers of JSString::value() should check for exceptions thereafter.
https://bugs.webkit.org/show_bug.cgi?id=154346
Reviewed by Geoffrey Garen.
Source/_javascript_Core:
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 a caller does 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.
* jsc.cpp:
(functionRun):
(functionLoad):
(functionReadFile):
(functionCheckSyntax):
(functionLoadWebAssembly):
(functionLoadModule):
(functionCheckModuleSyntax):
* runtime/DateConstructor.cpp:
(JSC::dateParse):
(JSC::dateNow):
* runtime/JSGlobalObjectFunctions.cpp:
(JSC::globalFuncEval):
* tools/JSDollarVMPrototype.cpp:
(JSC::functionPrint):
Source/WebCore:
No new tests. The crash that results from this issue is dependent on a race
condition where an OutOfMemory error occurs precisely at the point where the
JSString::value() function is called on a rope JSString.
* bindings/js/JSHTMLAllCollectionCustom.cpp:
(WebCore::callHTMLAllCollection):
* bindings/js/JSStorageCustom.cpp:
(WebCore::JSStorage::putDelegate):
- Added a comment at the site of the exception check to clarify the meaning of
the return value.
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (196744 => 196745)
--- trunk/Source/_javascript_Core/ChangeLog 2016-02-18 06:13:15 UTC (rev 196744)
+++ trunk/Source/_javascript_Core/ChangeLog 2016-02-18 06:28:26 UTC (rev 196745)
@@ -1,3 +1,36 @@
+2016-02-17 Mark Lam <[email protected]>
+
+ Callers of JSString::value() should check for exceptions thereafter.
+ https://bugs.webkit.org/show_bug.cgi?id=154346
+
+ Reviewed by Geoffrey Garen.
+
+ 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 a caller does 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.
+
+ * jsc.cpp:
+ (functionRun):
+ (functionLoad):
+ (functionReadFile):
+ (functionCheckSyntax):
+ (functionLoadWebAssembly):
+ (functionLoadModule):
+ (functionCheckModuleSyntax):
+ * runtime/DateConstructor.cpp:
+ (JSC::dateParse):
+ (JSC::dateNow):
+ * runtime/JSGlobalObjectFunctions.cpp:
+ (JSC::globalFuncEval):
+ * tools/JSDollarVMPrototype.cpp:
+ (JSC::functionPrint):
+
2016-02-17 Benjamin Poulain <[email protected]>
[JSC] ARM64: Support the immediate format used for bit operations in Air
Modified: trunk/Source/_javascript_Core/jsc.cpp (196744 => 196745)
--- trunk/Source/_javascript_Core/jsc.cpp 2016-02-18 06:13:15 UTC (rev 196744)
+++ trunk/Source/_javascript_Core/jsc.cpp 2016-02-18 06:28:26 UTC (rev 196745)
@@ -1243,6 +1243,8 @@
EncodedJSValue JSC_HOST_CALL functionRun(ExecState* exec)
{
String fileName = exec->argument(0).toString(exec)->value(exec);
+ if (exec->hadException())
+ return JSValue::encode(jsUndefined());
Vector<char> script;
if (!fetchScriptFromLocalFileSystem(fileName, script))
return JSValue::encode(exec->vm().throwException(exec, createError(exec, ASCIILiteral("Could not open file."))));
@@ -1272,6 +1274,8 @@
EncodedJSValue JSC_HOST_CALL functionLoad(ExecState* exec)
{
String fileName = exec->argument(0).toString(exec)->value(exec);
+ if (exec->hadException())
+ return JSValue::encode(jsUndefined());
Vector<char> script;
if (!fetchScriptFromLocalFileSystem(fileName, script))
return JSValue::encode(exec->vm().throwException(exec, createError(exec, ASCIILiteral("Could not open file."))));
@@ -1288,6 +1292,8 @@
EncodedJSValue JSC_HOST_CALL functionReadFile(ExecState* exec)
{
String fileName = exec->argument(0).toString(exec)->value(exec);
+ if (exec->hadException())
+ return JSValue::encode(jsUndefined());
Vector<char> script;
if (!fillBufferWithContentsOfFile(fileName, script))
return JSValue::encode(exec->vm().throwException(exec, createError(exec, ASCIILiteral("Could not open file."))));
@@ -1298,6 +1304,8 @@
EncodedJSValue JSC_HOST_CALL functionCheckSyntax(ExecState* exec)
{
String fileName = exec->argument(0).toString(exec)->value(exec);
+ if (exec->hadException())
+ return JSValue::encode(jsUndefined());
Vector<char> script;
if (!fetchScriptFromLocalFileSystem(fileName, script))
return JSValue::encode(exec->vm().throwException(exec, createError(exec, ASCIILiteral("Could not open file."))));
@@ -1565,6 +1573,8 @@
EncodedJSValue JSC_HOST_CALL functionLoadWebAssembly(ExecState* exec)
{
String fileName = exec->argument(0).toString(exec)->value(exec);
+ if (exec->hadException())
+ return JSValue::encode(jsUndefined());
Vector<char> buffer;
if (!fillBufferWithContentsOfFile(fileName, buffer))
return JSValue::encode(exec->vm().throwException(exec, createError(exec, ASCIILiteral("Could not open file."))));
@@ -1584,6 +1594,8 @@
EncodedJSValue JSC_HOST_CALL functionLoadModule(ExecState* exec)
{
String fileName = exec->argument(0).toString(exec)->value(exec);
+ if (exec->hadException())
+ return JSValue::encode(jsUndefined());
Vector<char> script;
if (!fetchScriptFromLocalFileSystem(fileName, script))
return JSValue::encode(exec->vm().throwException(exec, createError(exec, ASCIILiteral("Could not open file."))));
@@ -1608,6 +1620,8 @@
EncodedJSValue JSC_HOST_CALL functionCheckModuleSyntax(ExecState* exec)
{
String source = exec->argument(0).toString(exec)->value(exec);
+ if (exec->hadException())
+ return JSValue::encode(jsUndefined());
StopWatch stopWatch;
stopWatch.start();
Modified: trunk/Source/_javascript_Core/runtime/DateConstructor.cpp (196744 => 196745)
--- trunk/Source/_javascript_Core/runtime/DateConstructor.cpp 2016-02-18 06:13:15 UTC (rev 196744)
+++ trunk/Source/_javascript_Core/runtime/DateConstructor.cpp 2016-02-18 06:28:26 UTC (rev 196745)
@@ -205,7 +205,10 @@
EncodedJSValue JSC_HOST_CALL dateParse(ExecState* exec)
{
- return JSValue::encode(jsNumber(parseDate(exec->vm(), exec->argument(0).toString(exec)->value(exec))));
+ String dateStr = exec->argument(0).toString(exec)->value(exec);
+ if (exec->hadException())
+ return JSValue::encode(jsUndefined());
+ return JSValue::encode(jsNumber(parseDate(exec->vm(), dateStr)));
}
EncodedJSValue JSC_HOST_CALL dateNow(ExecState* exec)
Modified: trunk/Source/_javascript_Core/runtime/JSGlobalObjectFunctions.cpp (196744 => 196745)
--- trunk/Source/_javascript_Core/runtime/JSGlobalObjectFunctions.cpp 2016-02-18 06:13:15 UTC (rev 196744)
+++ trunk/Source/_javascript_Core/runtime/JSGlobalObjectFunctions.cpp 2016-02-18 06:28:26 UTC (rev 196745)
@@ -574,6 +574,8 @@
}
String s = x.toString(exec)->value(exec);
+ if (exec->hadException())
+ return JSValue::encode(jsUndefined());
if (s.is8Bit()) {
LiteralParser<LChar> preparser(exec, s.characters8(), s.length(), NonStrictJSON);
Modified: trunk/Source/_javascript_Core/tools/JSDollarVMPrototype.cpp (196744 => 196745)
--- trunk/Source/_javascript_Core/tools/JSDollarVMPrototype.cpp 2016-02-18 06:13:15 UTC (rev 196744)
+++ trunk/Source/_javascript_Core/tools/JSDollarVMPrototype.cpp 2016-02-18 06:28:26 UTC (rev 196745)
@@ -303,7 +303,10 @@
for (unsigned i = 0; i < exec->argumentCount(); ++i) {
if (i)
dataLog(" ");
- dataLog(exec->uncheckedArgument(i).toString(exec)->value(exec));
+ String argStr = exec->uncheckedArgument(i).toString(exec)->value(exec);
+ if (exec->hadException())
+ return JSValue::encode(jsUndefined());
+ dataLog(argStr);
}
return JSValue::encode(jsUndefined());
}
Modified: trunk/Source/WebCore/ChangeLog (196744 => 196745)
--- trunk/Source/WebCore/ChangeLog 2016-02-18 06:13:15 UTC (rev 196744)
+++ trunk/Source/WebCore/ChangeLog 2016-02-18 06:28:26 UTC (rev 196745)
@@ -1,3 +1,21 @@
+2016-02-17 Mark Lam <[email protected]>
+
+ Callers of JSString::value() should check for exceptions thereafter.
+ https://bugs.webkit.org/show_bug.cgi?id=154346
+
+ Reviewed by Geoffrey Garen.
+
+ No new tests. The crash that results from this issue is dependent on a race
+ condition where an OutOfMemory error occurs precisely at the point where the
+ JSString::value() function is called on a rope JSString.
+
+ * bindings/js/JSHTMLAllCollectionCustom.cpp:
+ (WebCore::callHTMLAllCollection):
+ * bindings/js/JSStorageCustom.cpp:
+ (WebCore::JSStorage::putDelegate):
+ - Added a comment at the site of the exception check to clarify the meaning of
+ the return value.
+
2016-02-17 David Kilzer <[email protected]>
[Cocoa] Always check the return value of dlopen() and dlsym() in Release builds
Modified: trunk/Source/WebCore/bindings/js/JSHTMLAllCollectionCustom.cpp (196744 => 196745)
--- trunk/Source/WebCore/bindings/js/JSHTMLAllCollectionCustom.cpp 2016-02-18 06:13:15 UTC (rev 196744)
+++ trunk/Source/WebCore/bindings/js/JSHTMLAllCollectionCustom.cpp 2016-02-18 06:28:26 UTC (rev 196745)
@@ -65,6 +65,8 @@
if (exec->argumentCount() == 1) {
// Support for document.all(<index>) etc.
String string = exec->argument(0).toString(exec)->value(exec);
+ if (exec->hadException())
+ return JSValue::encode(jsUndefined());
if (Optional<uint32_t> index = parseIndex(*string.impl()))
return JSValue::encode(toJS(exec, jsCollection->globalObject(), collection.item(index.value())));
@@ -74,6 +76,8 @@
// The second arg, if set, is the index of the item we want
String string = exec->argument(0).toString(exec)->value(exec);
+ if (exec->hadException())
+ return JSValue::encode(jsUndefined());
if (Optional<uint32_t> index = parseIndex(*exec->argument(1).toWTFString(exec).impl())) {
if (auto* item = collection.namedItemWithIndex(string, index.value()))
return JSValue::encode(toJS(exec, jsCollection->globalObject(), item));
Modified: trunk/Source/WebCore/bindings/js/JSStorageCustom.cpp (196744 => 196745)
--- trunk/Source/WebCore/bindings/js/JSStorageCustom.cpp 2016-02-18 06:13:15 UTC (rev 196744)
+++ trunk/Source/WebCore/bindings/js/JSStorageCustom.cpp 2016-02-18 06:28:26 UTC (rev 196745)
@@ -114,8 +114,13 @@
return false;
String stringValue = value.toString(exec)->value(exec);
- if (exec->hadException())
+ if (exec->hadException()) {
+ // The return value indicates whether putDelegate() should handle the put operation (which
+ // if true, tells the caller not to execute the generic put). It does not indicate whether
+ // putDelegate() did successfully complete the operation or not (which it didn't in this
+ // case due to the exception).
return true;
+ }
ExceptionCode ec = 0;
wrapped().setItem(propertyNameToString(propertyName), stringValue, ec);
