Bug 83780 – negative length applied to Array#slice

• Component: JavaScriptCore
• Status: RESOLVED WORKSFORME

[Andrea Giammarchi]

this through the url bar or the console: [].slice.call({length:-1})

and bye bye browser ... in Firefox instant exception, in Opera.Next an Array with maximum possible length, both Webkit Nightly, Safari, and Chrome Canary get stuck.

Not sure if this is related to the loop here ... unsigned casts are all over the place so probably is this part that has something wrong ?
resObj->setLength(exec, n);
return JSValue::encode(result);

This is out of:
// http://svn.webkit.org/repository/webkit/trunk/Source/JavaScriptCore/runtime/ArrayPrototype.cpp

EncodedJSValue JSC_HOST_CALL arrayProtoFuncSlice(ExecState* exec)
{
    // http://developer.netscape.com/docs/manuals/js/client/jsref/array.htm#1193713 or 15.4.4.10
    JSObject* thisObj = exec->hostThisValue().toObject(exec);
    unsigned length = thisObj->get(exec, exec->propertyNames().length).toUInt32(exec);
    if (exec->hadException())
        return JSValue::encode(jsUndefined());

    // We return a new array
    JSArray* resObj = constructEmptyArray(exec);
    JSValue result = resObj;

    unsigned begin = argumentClampedIndexFromStartOrEnd(exec, 0, length);
    unsigned end = argumentClampedIndexFromStartOrEnd(exec, 1, length, length);

    unsigned n = 0;
    for (unsigned k = begin; k < end; k++, n++) {
        JSValue v = getProperty(exec, thisObj, k);
        if (exec->hadException())
            return JSValue::encode(jsUndefined());
        if (v)
            resObj->putDirectIndex(exec, n, v);
    }
    resObj->setLength(exec, n);
    return JSValue::encode(result);
}

[Gavin Barraclough]

I think JSC is correct here.

The toUInt32 conversion is spec defined behavior (see 15.4.4.10 step 4), so your code fragment is asking the engine to inspect the this object for four billion possible properties.  This takes a while. :-)

JSC does not provide a mechanism asynchronously interrupt execution; instead we rely on the browser killing the web process if it's not interested in waiting for the script to complete.  If you try navigating in Safari you should be given the option to do so.

[Andrea Giammarchi]

I understand ... (ad this was quite old too) but in this way is too easy to crash a browser via malicious code. Don't you think? :-)

(In reply to comment #1)
> I think JSC is correct here.
> 
> The toUInt32 conversion is spec defined behavior (see 15.4.4.10 step 4), so your code fragment is asking the engine to inspect the this object for four billion possible properties.  This takes a while. :-)
> 
> JSC does not provide a mechanism asynchronously interrupt execution; instead we rely on the browser killing the web process if it's not interested in waiting for the script to complete.  If you try navigating in Safari you should be given the option to do so.

[Gavin Barraclough]

> I understand ... (ad this was quite old too) but in this way is too easy to crash a browser via malicious code. Don't you think? :-)

With WebKit2 this does not actually crash the browser - the page will just take a long time loading; there are plenty of equally easy ways to write a web page that will take a long time to load, e.g. "while(1);" ;-)  In response to a slow loading page, the browser can choose to prompt to give the user the option of either waiting for the load to complete or to interrupt the load.  Killing the web process is a great way to asynchronously interrupt the load since it firmly guarantees all resources will be cleaned up correctly.

The approach of killing the web process does have a keep drawback, in that it currently affects all tabs open in the browser.  In the case where the user wishes to wait for the load, all other pages will also be held up.  Where the user chooses to interrupt the load, this will force all pages to reload.  The drawback is really associated with the fact that multiple tabs are using the same web processes, and the fix is likely to change this.