Commit 0004733c authored by jbroman's avatar jbroman Committed by Commit bot

ValueSerializer: Add more checks before trying to allocate memory for a dense array.

Found with libfuzzer. The length is automatically converted to int (thus
large sizes could become negative, even though they are legal "array sizes").
Besides that, the length is coerced to a SMI (which is an even tighter
constraint on 32-bit systems, where it limits the legal sizes to 2^30 - 1).

Add checks that the length of a dense array is below that threshold, and also
fail fast if a length that is provided obviously could not be the correct dense
length (because there isn't enough data left in the buffer to populate such an
array).

BUG=chromium:148757

Review-Url: https://codereview.chromium.org/2399873002
Cr-Commit-Position: refs/heads/master@{#40094}
parent 58529ed3
......@@ -470,6 +470,8 @@ Maybe<bool> ValueSerializer::WriteJSArray(Handle<JSArray> array) {
array->HasFastElements() && !array->HasFastHoleyElements();
if (should_serialize_densely) {
DCHECK_LE(length, static_cast<uint32_t>(FixedArray::kMaxLength));
// TODO(jbroman): Distinguish between undefined and a hole (this can happen
// if serializing one of the elements deletes another). This requires wire
// format changes.
......@@ -1165,8 +1167,15 @@ MaybeHandle<JSArray> ValueDeserializer::ReadDenseJSArray() {
// If we are at the end of the stack, abort. This function may recurse.
STACK_CHECK(isolate_, MaybeHandle<JSArray>());
// We shouldn't permit an array larger than the biggest we can request from
// V8. As an additional sanity check, since each entry will take at least one
// byte to encode, if there are fewer bytes than that we can also fail fast.
uint32_t length;
if (!ReadVarint<uint32_t>().To(&length)) return MaybeHandle<JSArray>();
if (!ReadVarint<uint32_t>().To(&length) ||
length > static_cast<uint32_t>(FixedArray::kMaxLength) ||
length > static_cast<size_t>(end_ - position_)) {
return MaybeHandle<JSArray>();
}
uint32_t id = next_id_++;
HandleScope scope(isolate_);
......
......@@ -977,6 +977,14 @@ TEST_F(ValueSerializerTest, DecodeArray) {
});
}
TEST_F(ValueSerializerTest, DecodeInvalidOverLargeArray) {
// So large it couldn't exist in the V8 heap, and its size couldn't fit in a
// SMI on 32-bit systems (2^30).
InvalidDecodeTest({0xff, 0x09, 0x41, 0x80, 0x80, 0x80, 0x80, 0x04});
// Not so large, but there isn't enough data left in the buffer.
InvalidDecodeTest({0xff, 0x09, 0x41, 0x01});
}
TEST_F(ValueSerializerTest, RoundTripArrayWithNonEnumerableElement) {
// Even though this array looks like [1,5,3], the 5 should be missing from the
// perspective of structured clone, which only clones properties that were
......
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