From 077406dc36e494366c3a3ece98638028554a1c6c Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Tue, 30 Mar 2021 13:32:19 +0200 Subject: [PATCH] LibJS: Fix two issues with array (length > INT32_MAX) 1. Allow Value(size_t) and use it for array length properties. If an array length can't fit in an Int32 value, we shouldn't go out of or way to force it into one. Instead, for values above INT32_MAX, we simply store them as Double values. 2. Switch to generic indexed property storage for large arrays. Previously we would always allocate array storage eagerly when the length property was set. This meant that "a.length = 0x80000000" would trivially DOS the engine on 32-bit since we don't have that much VM. We now switch to generic storage when changing the length moves us over the 4M entry mark. Fixes #5986. --- Userland/Libraries/LibJS/Runtime/Array.cpp | 2 +- .../Libraries/LibJS/Runtime/IndexedProperties.cpp | 12 ++++++++++++ Userland/Libraries/LibJS/Runtime/Value.h | 11 +++++++++++ .../Tests/builtins/Array/array-length-setter.js | 10 ++++++++++ 4 files changed, 34 insertions(+), 1 deletion(-) diff --git a/Userland/Libraries/LibJS/Runtime/Array.cpp b/Userland/Libraries/LibJS/Runtime/Array.cpp index dabf19f9fb..c04a65c1d8 100644 --- a/Userland/Libraries/LibJS/Runtime/Array.cpp +++ b/Userland/Libraries/LibJS/Runtime/Array.cpp @@ -69,7 +69,7 @@ JS_DEFINE_NATIVE_GETTER(Array::length_getter) auto* array = typed_this(vm, global_object); if (!array) return {}; - return Value(static_cast(array->indexed_properties().array_like_size())); + return Value(array->indexed_properties().array_like_size()); } JS_DEFINE_NATIVE_SETTER(Array::length_setter) diff --git a/Userland/Libraries/LibJS/Runtime/IndexedProperties.cpp b/Userland/Libraries/LibJS/Runtime/IndexedProperties.cpp index 5f285838d8..4993c9cf8f 100644 --- a/Userland/Libraries/LibJS/Runtime/IndexedProperties.cpp +++ b/Userland/Libraries/LibJS/Runtime/IndexedProperties.cpp @@ -333,6 +333,18 @@ void IndexedProperties::append_all(Object* this_object, const IndexedProperties& void IndexedProperties::set_array_like_size(size_t new_size) { + constexpr size_t length_setter_generic_storage_threshold = 4 * MiB; + auto current_array_like_size = array_like_size(); + + // We can't use simple storage for lengths that don't fit in an i32. + // Also, to avoid gigantic unused storage allocations, let's put an (arbitrary) 4M cap on simple storage here. + // This prevents something like "a = []; a.length = 0x80000000;" from allocating 2G entries. + if (m_storage->is_simple_storage() + && (new_size > NumericLimits::max() + || (current_array_like_size < length_setter_generic_storage_threshold && new_size > length_setter_generic_storage_threshold))) { + switch_to_generic_storage(); + } + m_storage->set_array_like_size(new_size); } diff --git a/Userland/Libraries/LibJS/Runtime/Value.h b/Userland/Libraries/LibJS/Runtime/Value.h index d0e51bc2e1..e763cfa0bf 100644 --- a/Userland/Libraries/LibJS/Runtime/Value.h +++ b/Userland/Libraries/LibJS/Runtime/Value.h @@ -122,6 +122,17 @@ public: } } + explicit Value(unsigned long value) + { + if (value > NumericLimits::max()) { + m_value.as_double = static_cast(value); + m_type = Type::Double; + } else { + m_value.as_i32 = static_cast(value); + m_type = Type::Int32; + } + } + explicit Value(unsigned value) { if (value > NumericLimits::max()) { diff --git a/Userland/Libraries/LibJS/Tests/builtins/Array/array-length-setter.js b/Userland/Libraries/LibJS/Tests/builtins/Array/array-length-setter.js index 9a1043f5d4..662fd6e04e 100644 --- a/Userland/Libraries/LibJS/Tests/builtins/Array/array-length-setter.js +++ b/Userland/Libraries/LibJS/Tests/builtins/Array/array-length-setter.js @@ -34,4 +34,14 @@ describe("normal behavior", () => { a.length = true; expect(a).toHaveLength(1); }); + + test("setting a huge array length", () => { + var a = []; + a.length = 0x80000000; + expect(a.length).toEqual(0x80000000); + + var b = []; + b.length = 0x80000001; + expect(b.length).toEqual(0x80000001); + }); });