Changeset 196745 in webkit


Ignore:
Timestamp:
Feb 17, 2016 10:28:26 PM (8 years ago)
Author:
mark.lam@apple.com
Message:

Callers of JSString::value() should check for exceptions thereafter.
​https://bugs.webkit.org/show_bug.cgi?id=154346

Reviewed by Geoffrey Garen.

Source/JavaScriptCore:

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.
Location:
trunk/Source
Files:
8 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/JavaScriptCore/ChangeLog

    r196736 r196745  
     12016-02-17  Mark Lam  <mark.lam@apple.com>
     2
     3        Callers of JSString::value() should check for exceptions thereafter.
     4        https://bugs.webkit.org/show_bug.cgi?id=154346
     5
     6        Reviewed by Geoffrey Garen.
     7
     8        JSString::value() can throw an exception if the JS string is a rope and value()
     9        needs to resolve the rope but encounters an OutOfMemory error.  If value() is not
     10        able to resolve the rope, it will return a null string (in addition to throwing
     11        the exception).  If a caller does not check for exceptions after calling
     12        JSString::value(), they may eventually use the returned null string and crash the
     13        VM.
     14
     15        The fix is to add all the necessary exception checks, and do the appropriate
     16        handling if needed.
     17
     18        * jsc.cpp:
     19        (functionRun):
     20        (functionLoad):
     21        (functionReadFile):
     22        (functionCheckSyntax):
     23        (functionLoadWebAssembly):
     24        (functionLoadModule):
     25        (functionCheckModuleSyntax):
     26        * runtime/DateConstructor.cpp:
     27        (JSC::dateParse):
     28        (JSC::dateNow):
     29        * runtime/JSGlobalObjectFunctions.cpp:
     30        (JSC::globalFuncEval):
     31        * tools/JSDollarVMPrototype.cpp:
     32        (JSC::functionPrint):
     33
    1342016-02-17  Benjamin Poulain  <bpoulain@apple.com>
    235

  • trunk/Source/JavaScriptCore/jsc.cpp

    r196331 r196745  
    12441244{
    12451245    String fileName = exec->argument(0).toString(exec)->value(exec);
     1246    if (exec->hadException())
     1247        return JSValue::encode(jsUndefined());
    12461248    Vector<char> script;
    12471249    if (!fetchScriptFromLocalFileSystem(fileName, script))
    … …  
    12731275{
    12741276    String fileName = exec->argument(0).toString(exec)->value(exec);
     1277    if (exec->hadException())
     1278        return JSValue::encode(jsUndefined());
    12751279    Vector<char> script;
    12761280    if (!fetchScriptFromLocalFileSystem(fileName, script))
    … …  
    12891293{
    12901294    String fileName = exec->argument(0).toString(exec)->value(exec);
     1295    if (exec->hadException())
     1296        return JSValue::encode(jsUndefined());
    12911297    Vector<char> script;
    12921298    if (!fillBufferWithContentsOfFile(fileName, script))
    … …  
    12991305{
    13001306    String fileName = exec->argument(0).toString(exec)->value(exec);
     1307    if (exec->hadException())
     1308        return JSValue::encode(jsUndefined());
    13011309    Vector<char> script;
    13021310    if (!fetchScriptFromLocalFileSystem(fileName, script))
    … …  
    15661574{
    15671575    String fileName = exec->argument(0).toString(exec)->value(exec);
     1576    if (exec->hadException())
     1577        return JSValue::encode(jsUndefined());
    15681578    Vector<char> buffer;
    15691579    if (!fillBufferWithContentsOfFile(fileName, buffer))
    … …  
    15851595{
    15861596    String fileName = exec->argument(0).toString(exec)->value(exec);
     1597    if (exec->hadException())
     1598        return JSValue::encode(jsUndefined());
    15871599    Vector<char> script;
    15881600    if (!fetchScriptFromLocalFileSystem(fileName, script))
    … …  
    16091621{
    16101622    String source = exec->argument(0).toString(exec)->value(exec);
     1623    if (exec->hadException())
     1624        return JSValue::encode(jsUndefined());
    16111625
    16121626    StopWatch stopWatch;

  • trunk/Source/JavaScriptCore/runtime/DateConstructor.cpp

    r194863 r196745  
    206206EncodedJSValue JSC_HOST_CALL dateParse(ExecState* exec)
    207207{
    208     return JSValue::encode(jsNumber(parseDate(exec->vm(), exec->argument(0).toString(exec)->value(exec))));
     208    String dateStr = exec->argument(0).toString(exec)->value(exec);
     209    if (exec->hadException())
     210        return JSValue::encode(jsUndefined());
     211    return JSValue::encode(jsNumber(parseDate(exec->vm(), dateStr)));
    209212}
    210213

  • trunk/Source/JavaScriptCore/runtime/JSGlobalObjectFunctions.cpp

    r194449 r196745  
    575575
    576576    String s = x.toString(exec)->value(exec);
     577    if (exec->hadException())
     578        return JSValue::encode(jsUndefined());
    577579
    578580    if (s.is8Bit()) {

  • trunk/Source/JavaScriptCore/tools/JSDollarVMPrototype.cpp

    r190555 r196745  
    304304        if (i)
    305305            dataLog(" ");
    306         dataLog(exec->uncheckedArgument(i).toString(exec)->value(exec));
     306        String argStr = exec->uncheckedArgument(i).toString(exec)->value(exec);
     307        if (exec->hadException())
     308            return JSValue::encode(jsUndefined());
     309        dataLog(argStr);
    307310    }
    308311    return JSValue::encode(jsUndefined());

  • trunk/Source/WebCore/ChangeLog

    r196744 r196745  
     12016-02-17  Mark Lam  <mark.lam@apple.com>
     2
     3        Callers of JSString::value() should check for exceptions thereafter.
     4        https://bugs.webkit.org/show_bug.cgi?id=154346
     5
     6        Reviewed by Geoffrey Garen.
     7
     8        No new tests.  The crash that results from this issue is dependent on a race
     9        condition where an OutOfMemory error occurs precisely at the point where the
     10        JSString::value() function is called on a rope JSString.
     11
     12        * bindings/js/JSHTMLAllCollectionCustom.cpp:
     13        (WebCore::callHTMLAllCollection):
     14        * bindings/js/JSStorageCustom.cpp:
     15        (WebCore::JSStorage::putDelegate):
     16        - Added a comment at the site of the exception check to clarify the meaning of
     17          the return value.
     18
    1192016-02-17  David Kilzer  <ddkilzer@apple.com>
    220

  • trunk/Source/WebCore/bindings/js/JSHTMLAllCollectionCustom.cpp

    r191887 r196745  
    6666        // Support for document.all(<index>) etc.
    6767        String string = exec->argument(0).toString(exec)->value(exec);
     68        if (exec->hadException())
     69            return JSValue::encode(jsUndefined());
    6870        if (Optional<uint32_t> index = parseIndex(*string.impl()))
    6971            return JSValue::encode(toJS(exec, jsCollection->globalObject(), collection.item(index.value())));
    … …  
    7577    // The second arg, if set, is the index of the item we want
    7678    String string = exec->argument(0).toString(exec)->value(exec);
     79    if (exec->hadException())
     80        return JSValue::encode(jsUndefined());
    7781    if (Optional<uint32_t> index = parseIndex(*exec->argument(1).toWTFString(exec).impl())) {
    7882        if (auto* item = collection.namedItemWithIndex(string, index.value()))

  • trunk/Source/WebCore/bindings/js/JSStorageCustom.cpp

    r196722 r196745  
    115115
    116116    String stringValue = value.toString(exec)->value(exec);
    117     if (exec->hadException())
     117    if (exec->hadException()) {
     118        // The return value indicates whether putDelegate() should handle the put operation (which
     119        // if true, tells the caller not to execute the generic put). It does not indicate whether
     120        // putDelegate() did successfully complete the operation or not (which it didn't in this
     121        // case due to the exception).
    118122        return true;
     123    }
    119124
    120125    ExceptionCode ec = 0;
Note: See TracChangeset for help on using the changeset viewer.