Title: [230101] trunk
- Revision
- 230101
- Author
- rmoris...@apple.com
- Date
- 2018-03-30 05:05:34 -0700 (Fri, 30 Mar 2018)
Log Message
Out-of-bounds accesses due to a missing check for MAX_STORAGE_VECTOR_LENGTH in unshiftCountForAnyIndexingType
https://bugs.webkit.org/show_bug.cgi?id=183657
JSTests:
Reviewed by Keith Miller.
* stress/large-unshift-splice.js: Added.
(make_contig_arr):
Source/_javascript_Core:
<rdar://problem/38464399>
Reviewed by Keith Miller.
There was just a missing check in unshiftCountForIndexingType.
I've also replaced 'return false' by 'return true' in the case of an 'out-of-memory' exception, because 'return false' means 'please continue to the slow path',
and the slow path has an assert that there is no unhandled exception (line 360 of ArrayPrototype.cpp).
Finally, I made the assert in ensureLength a release assert as it would have caught this bug and prevented it from being a security risk.
* runtime/ArrayPrototype.cpp:
(JSC::unshift):
* runtime/JSArray.cpp:
(JSC::JSArray::unshiftCountWithAnyIndexingType):
* runtime/JSObject.h:
(JSC::JSObject::ensureLength):
Modified Paths
Added Paths
Diff
Modified: trunk/JSTests/ChangeLog (230100 => 230101)
--- trunk/JSTests/ChangeLog 2018-03-30 05:41:47 UTC (rev 230100)
+++ trunk/JSTests/ChangeLog 2018-03-30 12:05:34 UTC (rev 230101)
@@ -1,3 +1,13 @@
+2018-03-30 Robin Morisset <rmoris...@apple.com>
+
+ Out-of-bounds accesses due to a missing check for MAX_STORAGE_VECTOR_LENGTH in unshiftCountForAnyIndexingType
+ https://bugs.webkit.org/show_bug.cgi?id=183657
+
+ Reviewed by Keith Miller.
+
+ * stress/large-unshift-splice.js: Added.
+ (make_contig_arr):
+
2018-03-28 Robin Morisset <rmoris...@apple.com>
appendQuotedJSONString stops on arithmetic overflow instead of propagating it upwards
Added: trunk/JSTests/stress/large-unshift-splice.js (0 => 230101)
--- trunk/JSTests/stress/large-unshift-splice.js (rev 0)
+++ trunk/JSTests/stress/large-unshift-splice.js 2018-03-30 12:05:34 UTC (rev 230101)
@@ -0,0 +1,16 @@
+//@ skip if $memoryLimited
+
+function make_contig_arr(sz)
+{
+ let a = [];
+ while (a.length < sz / 8)
+ a.push(3.14);
+ a.length *= 8;
+ return a;
+}
+
+let ARRAY_LENGTH = 0x10000000;
+let a = make_contig_arr(ARRAY_LENGTH);
+let b = make_contig_arr(0xff00);
+b.unshift(a.length-0x10000, 0);
+Array.prototype.splice.apply(a, b);
Modified: trunk/Source/_javascript_Core/ChangeLog (230100 => 230101)
--- trunk/Source/_javascript_Core/ChangeLog 2018-03-30 05:41:47 UTC (rev 230100)
+++ trunk/Source/_javascript_Core/ChangeLog 2018-03-30 12:05:34 UTC (rev 230101)
@@ -1,3 +1,23 @@
+2018-03-30 Robin Morisset <rmoris...@apple.com>
+
+ Out-of-bounds accesses due to a missing check for MAX_STORAGE_VECTOR_LENGTH in unshiftCountForAnyIndexingType
+ https://bugs.webkit.org/show_bug.cgi?id=183657
+ <rdar://problem/38464399>
+
+ Reviewed by Keith Miller.
+
+ There was just a missing check in unshiftCountForIndexingType.
+ I've also replaced 'return false' by 'return true' in the case of an 'out-of-memory' exception, because 'return false' means 'please continue to the slow path',
+ and the slow path has an assert that there is no unhandled exception (line 360 of ArrayPrototype.cpp).
+ Finally, I made the assert in ensureLength a release assert as it would have caught this bug and prevented it from being a security risk.
+
+ * runtime/ArrayPrototype.cpp:
+ (JSC::unshift):
+ * runtime/JSArray.cpp:
+ (JSC::JSArray::unshiftCountWithAnyIndexingType):
+ * runtime/JSObject.h:
+ (JSC::JSObject::ensureLength):
+
2018-03-29 Mark Lam <mark....@apple.com>
Add some pointer profiling support to B3 and Air.
Modified: trunk/Source/_javascript_Core/runtime/ArrayPrototype.cpp (230100 => 230101)
--- trunk/Source/_javascript_Core/runtime/ArrayPrototype.cpp 2018-03-30 05:41:47 UTC (rev 230100)
+++ trunk/Source/_javascript_Core/runtime/ArrayPrototype.cpp 2018-03-30 12:05:34 UTC (rev 230101)
@@ -348,7 +348,7 @@
RELEASE_ASSERT(currentCount <= (length - header));
// Guard against overflow.
- if (count > (UINT_MAX - length)) {
+ if (count > UINT_MAX - length) {
throwOutOfMemoryError(exec, scope);
return;
}
Modified: trunk/Source/_javascript_Core/runtime/JSArray.cpp (230100 => 230101)
--- trunk/Source/_javascript_Core/runtime/JSArray.cpp 2018-03-30 05:41:47 UTC (rev 230100)
+++ trunk/Source/_javascript_Core/runtime/JSArray.cpp 2018-03-30 12:05:34 UTC (rev 230101)
@@ -1060,10 +1060,13 @@
scope.release();
return unshiftCountWithArrayStorage(exec, startIndex, count, ensureArrayStorage(vm));
}
-
+
+ if (oldLength + count > MAX_STORAGE_VECTOR_LENGTH)
+ return false;
+
if (!ensureLength(vm, oldLength + count)) {
throwOutOfMemoryError(exec, scope);
- return false;
+ return true;
}
butterfly = this->butterfly();
@@ -1104,10 +1107,13 @@
scope.release();
return unshiftCountWithArrayStorage(exec, startIndex, count, ensureArrayStorage(vm));
}
-
+
+ if (oldLength + count > MAX_STORAGE_VECTOR_LENGTH)
+ return false;
+
if (!ensureLength(vm, oldLength + count)) {
throwOutOfMemoryError(exec, scope);
- return false;
+ return true;
}
butterfly = this->butterfly();
Modified: trunk/Source/_javascript_Core/runtime/JSObject.h (230100 => 230101)
--- trunk/Source/_javascript_Core/runtime/JSObject.h 2018-03-30 05:41:47 UTC (rev 230100)
+++ trunk/Source/_javascript_Core/runtime/JSObject.h 2018-03-30 12:05:34 UTC (rev 230101)
@@ -982,7 +982,7 @@
// the array is contiguous.
bool WARN_UNUSED_RETURN ensureLength(VM& vm, unsigned length)
{
- ASSERT(length <= MAX_STORAGE_VECTOR_LENGTH);
+ RELEASE_ASSERT(length <= MAX_STORAGE_VECTOR_LENGTH);
ASSERT(hasContiguous(indexingType()) || hasInt32(indexingType()) || hasDouble(indexingType()) || hasUndecided(indexingType()));
if (m_butterfly->vectorLength() < length) {
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
