From e26cfd313e61ebd26d1e0c7e71820f6c266be973 Mon Sep 17 00:00:00 2001 From: Robert Stefanic Date: Fri, 13 Aug 2021 12:59:57 -0400 Subject: [PATCH] LibJS: Prevent stack overflow in flatten_into_array() The check for stack space in VM from push_execution_context has been moved to a method on VM called did_reach_stack_space_limit. This allows us to check the stack size in other places besides push_execution_context. We can now verify that we have enough space on the stack before calling flatten_into_array to ensure that we don't cause a stack overflow error when calling the function with a large depth. --- Userland/Libraries/LibJS/Runtime/ArrayPrototype.cpp | 5 +++++ Userland/Libraries/LibJS/Runtime/VM.h | 9 +++++++-- .../LibJS/Tests/builtins/Array/Array.prototype.flat.js | 10 ++++++++++ 3 files changed, 22 insertions(+), 2 deletions(-) diff --git a/Userland/Libraries/LibJS/Runtime/ArrayPrototype.cpp b/Userland/Libraries/LibJS/Runtime/ArrayPrototype.cpp index 9e80b65e09..435908b7cf 100644 --- a/Userland/Libraries/LibJS/Runtime/ArrayPrototype.cpp +++ b/Userland/Libraries/LibJS/Runtime/ArrayPrototype.cpp @@ -1945,6 +1945,11 @@ static size_t flatten_into_array(GlobalObject& global_object, Object& new_array, } if (depth > 0 && value.is_array(global_object)) { + if (vm.did_reach_stack_space_limit()) { + vm.throw_exception(global_object, "Call stack size limit exceeded"); + return {}; + } + auto length = length_of_array_like(global_object, value.as_object()); if (vm.exception()) return {}; diff --git a/Userland/Libraries/LibJS/Runtime/VM.h b/Userland/Libraries/LibJS/Runtime/VM.h index 5e4fc8a563..7b2981c686 100644 --- a/Userland/Libraries/LibJS/Runtime/VM.h +++ b/Userland/Libraries/LibJS/Runtime/VM.h @@ -105,12 +105,17 @@ public: return *m_single_ascii_character_strings[character]; } + bool did_reach_stack_space_limit() const + { + // Note: the 32 kiB used to be 16 kiB, but that turned out to not be enough with ASAN enabled. + return m_stack_info.size_free() < 32 * KiB; + } + void push_execution_context(ExecutionContext& context, GlobalObject& global_object) { VERIFY(!exception()); // Ensure we got some stack space left, so the next function call doesn't kill us. - // Note: the 32 kiB used to be 16 kiB, but that turned out to not be enough with ASAN enabled. - if (m_stack_info.size_free() < 32 * KiB) + if (did_reach_stack_space_limit()) throw_exception(global_object, "Call stack size limit exceeded"); else m_execution_context_stack.append(&context); diff --git a/Userland/Libraries/LibJS/Tests/builtins/Array/Array.prototype.flat.js b/Userland/Libraries/LibJS/Tests/builtins/Array/Array.prototype.flat.js index faa4e2d0b4..3edae0f8e6 100644 --- a/Userland/Libraries/LibJS/Tests/builtins/Array/Array.prototype.flat.js +++ b/Userland/Libraries/LibJS/Tests/builtins/Array/Array.prototype.flat.js @@ -2,6 +2,16 @@ test("length is 0", () => { expect(Array.prototype.flat).toHaveLength(0); }); +describe("error", () => { + test("Issue #9317, stack overflow in flatten_into_array from flat call", () => { + var a = []; + a[0] = a; + expect(() => { + a.flat(3893232121); + }).toThrowWithMessage(Error, "Call stack size limit exceeded"); + }); +}); + describe("normal behavior", () => { test("basic functionality", () => { var array1 = [1, 2, [3, 4]];