From a5bf6cfff98a52ecceaace5dd8ccc8fd7e79ac90 Mon Sep 17 00:00:00 2001 From: Linus Groh Date: Sat, 10 Oct 2020 14:42:23 +0100 Subject: [PATCH] LibJS: Don't change offset when reconfiguring property in unique shape When changing the attributes of an existing property of an object with unique shape we must not change the PropertyMetadata offset. Doing so without resizing the underlying storage vector caused an OOB write crash. Fixes #3735. --- Libraries/LibJS/Runtime/Shape.cpp | 6 ++++-- .../Tests/builtins/Object/Object.defineProperty.js | 10 ++++++++++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/Libraries/LibJS/Runtime/Shape.cpp b/Libraries/LibJS/Runtime/Shape.cpp index 741e00e322..4fd26ed631 100644 --- a/Libraries/LibJS/Runtime/Shape.cpp +++ b/Libraries/LibJS/Runtime/Shape.cpp @@ -195,8 +195,10 @@ void Shape::reconfigure_property_in_unique_shape(const StringOrSymbol& property_ { ASSERT(is_unique()); ASSERT(m_property_table); - ASSERT(m_property_table->contains(property_name)); - m_property_table->set(property_name, { m_property_table->size(), attributes }); + auto it = m_property_table->find(property_name); + ASSERT(it != m_property_table->end()); + it->value.attributes = attributes; + m_property_table->set(property_name, it->value); } void Shape::remove_property_from_unique_shape(const StringOrSymbol& property_name, size_t offset) diff --git a/Libraries/LibJS/Tests/builtins/Object/Object.defineProperty.js b/Libraries/LibJS/Tests/builtins/Object/Object.defineProperty.js index ef72515569..bebe32997d 100644 --- a/Libraries/LibJS/Tests/builtins/Object/Object.defineProperty.js +++ b/Libraries/LibJS/Tests/builtins/Object/Object.defineProperty.js @@ -148,6 +148,16 @@ describe("normal functionality", () => { expect((o[s] = 5)).toBe(5); expect((o[s] = 4)).toBe(4); }); + + test("issue #3735, reconfiguring property in unique shape", () => { + const o = {}; + // In LibJS an object with more than 100 properties gets a unique shape + for (let i = 0; i < 101; ++i) { + o[`property${i}`] = i; + } + Object.defineProperty(o, "x", { configurable: true }); + Object.defineProperty(o, "x", { configurable: false }); + }); }); describe("errors", () => {