diff options
author | Luke Wilde <lukew@serenityos.org> | 2022-12-24 15:31:43 +0000 |
---|---|---|
committer | Andreas Kling <kling@serenityos.org> | 2022-12-25 15:32:51 +0100 |
commit | 7e701f6256432f11834413275d8d2dd78fa9320b (patch) | |
tree | 28e2c646279ec1d802c12ee682208f480a752cb4 /Userland/Libraries/LibWeb/HTML/Scripting | |
parent | 2334b4cebdcce1104d85afa75315c2721cc2df83 (diff) | |
download | serenity-7e701f6256432f11834413275d8d2dd78fa9320b.zip |
LibWeb: Keep unhandledrejection event promises alive when task is queued
This is fixed by making the "about to be notified rejected promises
list" use JS::Handle instead of JS::NonnullGCPtr. This UAF happens
because notify_about_rejected_promises makes a local copy of this list,
empties the member variable list and then moves the local copy into a
JS::SafeFunction lambda. JS::SafeFunction can only see GC pointers that
are in its storage, not external storage.
Example exploit (requires fixed microtask timing by removing the dummy
execution context):
```html
<script>
Promise.reject(new Error);
// Exit the script block, causing a microtask checkpoint and thus
// queuing of a task to fire the unhandled rejection event for the
// above promise.
// During the time after being queued but before being ran, these
// promises are not kept alive. This is because JS::SafeFunction cannot
// see into a Vector, meaning it can't visit the stored NonnullGCPtrs.
</script>
<script defer>
// Cause a garbage collection, destroying the above promise.
const b = [];
for (var i = 0; i < 200000; i++)
b.push({});
// Some time after this script block, the queued unhandled rejection
// event task will fire, with the event object containing the dead
// promise.
window.onunhandledrejection = (event) => {
let value = event.promise;
console.log(value);
}
</script>
```
Diffstat (limited to 'Userland/Libraries/LibWeb/HTML/Scripting')
-rw-r--r-- | Userland/Libraries/LibWeb/HTML/Scripting/Environments.cpp | 4 | ||||
-rw-r--r-- | Userland/Libraries/LibWeb/HTML/Scripting/Environments.h | 2 |
2 files changed, 2 insertions, 4 deletions
diff --git a/Userland/Libraries/LibWeb/HTML/Scripting/Environments.cpp b/Userland/Libraries/LibWeb/HTML/Scripting/Environments.cpp index 209b169313..40e6c46222 100644 --- a/Userland/Libraries/LibWeb/HTML/Scripting/Environments.cpp +++ b/Userland/Libraries/LibWeb/HTML/Scripting/Environments.cpp @@ -37,8 +37,6 @@ void EnvironmentSettingsObject::visit_edges(Cell::Visitor& visitor) { Base::visit_edges(visitor); visitor.visit(target_browsing_context); - for (auto& promise : m_about_to_be_notified_rejected_promises_list) - visitor.visit(promise); } JS::ExecutionContext& EnvironmentSettingsObject::realm_execution_context() @@ -203,7 +201,7 @@ bool EnvironmentSettingsObject::remove_from_outstanding_rejected_promises_weak_s void EnvironmentSettingsObject::push_onto_about_to_be_notified_rejected_promises_list(JS::NonnullGCPtr<JS::Promise> promise) { - m_about_to_be_notified_rejected_promises_list.append(move(promise)); + m_about_to_be_notified_rejected_promises_list.append(JS::make_handle(promise)); } bool EnvironmentSettingsObject::remove_from_about_to_be_notified_rejected_promises_list(JS::NonnullGCPtr<JS::Promise> promise) diff --git a/Userland/Libraries/LibWeb/HTML/Scripting/Environments.h b/Userland/Libraries/LibWeb/HTML/Scripting/Environments.h index bf270bedeb..e80f539546 100644 --- a/Userland/Libraries/LibWeb/HTML/Scripting/Environments.h +++ b/Userland/Libraries/LibWeb/HTML/Scripting/Environments.h @@ -133,7 +133,7 @@ private: Vector<JS::Promise*> m_outstanding_rejected_promises_weak_set; // https://html.spec.whatwg.org/multipage/webappapis.html#about-to-be-notified-rejected-promises-list - Vector<JS::NonnullGCPtr<JS::Promise>> m_about_to_be_notified_rejected_promises_list; + Vector<JS::Handle<JS::Promise>> m_about_to_be_notified_rejected_promises_list; }; EnvironmentSettingsObject& incumbent_settings_object(); |