Commit 404b4006 authored by barraclough@apple.com's avatar barraclough@apple.com

Bug 62405 - Fix integer overflow in Array.prototype.push

Reviewed by Oliver Hunt.

There are three integer overflows here, leading to safe (not a security risk)
but incorrect (non-spec-compliant) behaviour.

Two overflows occur when calculating the new length after pushing (one in the
fast version of push in JSArray, one in the generic version in ArrayPrototype).
The other occurs calculating indices to write to when multiple items are pushed.

These errors result in three test-262 failures.

Source/JavaScriptCore: 

* runtime/ArrayPrototype.cpp:
(JSC::arrayProtoFuncPush):
* runtime/JSArray.cpp:
(JSC::JSArray::put):
(JSC::JSArray::push):

LayoutTests: 

* sputnik/Conformance/15_Native_Objects/15.4_Array/15.4.4/15.4.4.7_Array_prototype_push/S15.4.4.7_A3-expected.txt:
* sputnik/Conformance/15_Native_Objects/15.4_Array/15.4.4/15.4.4.7_Array_prototype_push/S15.4.4.7_A4_T2-expected.txt:
* sputnik/Conformance/15_Native_Objects/15.4_Array/15.4.4/15.4.4.7_Array_prototype_push/S15.4.4.7_A4_T3-expected.txt:



git-svn-id: http://svn.webkit.org/repository/webkit/trunk@88503 268f45cc-cd09-0410-ab3c-d52691b4dbfc
parent 5d92606e
2011-06-09 Gavin Barraclough <barraclough@apple.com>
Reviewed by Oliver Hunt.
Bug 62405 - Fix integer overflow in Array.prototype.push
There are three integer overflows here, leading to safe (not a security risk)
but incorrect (non-spec-compliant) behaviour.
Two overflows occur when calculating the new length after pushing (one in the
fast version of push in JSArray, one in the generic version in ArrayPrototype).
The other occurs calculating indices to write to when multiple items are pushed.
These errors result in three test-262 failures.
* sputnik/Conformance/15_Native_Objects/15.4_Array/15.4.4/15.4.4.7_Array_prototype_push/S15.4.4.7_A3-expected.txt:
* sputnik/Conformance/15_Native_Objects/15.4_Array/15.4.4/15.4.4.7_Array_prototype_push/S15.4.4.7_A4_T2-expected.txt:
* sputnik/Conformance/15_Native_Objects/15.4_Array/15.4.4/15.4.4.7_Array_prototype_push/S15.4.4.7_A4_T3-expected.txt:
2011-06-09 Martin Robinson <mrobinson@igalia.com>
Reviewed by Eric Seidel.
......
S15.4.4.7_A3
FAIL SputnikError: #2.2: x = []; x.length = 4294967295; x.push("x") throw RangeError. Actual: SputnikError: #2.1: x = []; x.length = 4294967295; x.push("x") throw RangeError. Actual: 4294967295
PASS
TEST COMPLETE
S15.4.4.7_A4_T2
FAIL SputnikError: #1: var obj = {}; obj.push = Array.prototype.push; obj.length = 4294967295; obj.push("x", "y", "z") === 4294967298. Actual: 2
PASS
TEST COMPLETE
S15.4.4.7_A4_T3
FAIL SputnikError: #1: var obj = {}; obj.push = Array.prototype.push; obj.length = -1; obj.push("x", "y", "z") === 4294967298. Actual: 2
PASS
TEST COMPLETE
2011-06-09 Gavin Barraclough <barraclough@apple.com>
Reviewed by Oliver Hunt.
Bug 62405 - Fix integer overflow in Array.prototype.push
There are three integer overflows here, leading to safe (not a security risk)
but incorrect (non-spec-compliant) behaviour.
Two overflows occur when calculating the new length after pushing (one in the
fast version of push in JSArray, one in the generic version in ArrayPrototype).
The other occurs calculating indices to write to when multiple items are pushed.
These errors result in three test-262 failures.
* runtime/ArrayPrototype.cpp:
(JSC::arrayProtoFuncPush):
* runtime/JSArray.cpp:
(JSC::JSArray::put):
(JSC::JSArray::push):
2011-06-09 Dan Bernstein <mitz@apple.com>
Reviewed by Anders Carlsson.
......
......@@ -401,11 +401,19 @@ EncodedJSValue JSC_HOST_CALL arrayProtoFuncPush(ExecState* exec)
if (exec->hadException())
return JSValue::encode(jsUndefined());
for (unsigned n = 0; n < exec->argumentCount(); n++)
thisObj->put(exec, length + n, exec->argument(n));
length += exec->argumentCount();
putProperty(exec, thisObj, exec->propertyNames().length, jsNumber(length));
return JSValue::encode(jsNumber(length));
for (unsigned n = 0; n < exec->argumentCount(); n++) {
// Check for integer overflow; where safe we can do a fast put by index.
if (length + n >= length)
thisObj->put(exec, length + n, exec->argument(n));
else {
PutPropertySlot slot;
Identifier propertyName(exec, JSValue((int64_t)length + (int64_t)n).toString(exec));
thisObj->put(exec, propertyName, exec->argument(n), slot);
}
}
JSValue newLength = jsNumber((int64_t)length + (int64_t)exec->argumentCount());
putProperty(exec, thisObj, exec->propertyNames().length, newLength);
return JSValue::encode(newLength);
}
EncodedJSValue JSC_HOST_CALL arrayProtoFuncReverse(ExecState* exec)
......
......@@ -330,7 +330,7 @@ void JSArray::put(ExecState* exec, const Identifier& propertyName, JSValue value
if (propertyName == exec->propertyNames().length) {
unsigned newLength = value.toUInt32(exec);
if (value.toNumber(exec) != static_cast<double>(newLength)) {
throwError(exec, createRangeError(exec, "Invalid array length."));
throwError(exec, createRangeError(exec, "Invalid array length"));
return;
}
setLength(newLength);
......@@ -736,6 +736,12 @@ void JSArray::push(ExecState* exec, JSValue value)
ArrayStorage* storage = m_storage;
if (UNLIKELY(storage->m_length == 0xFFFFFFFFu)) {
put(exec, storage->m_length, value);
throwError(exec, createRangeError(exec, "Invalid array length"));
return;
}
if (storage->m_length < m_vectorLength) {
storage->m_vector[storage->m_length].set(exec->globalData(), this, value);
++storage->m_numValuesInVector;
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment