From 7e701f6256432f11834413275d8d2dd78fa9320b Mon Sep 17 00:00:00 2001
From: Luke Wilde <lukew@serenityos.org>
Date: Sat, 24 Dec 2022 15:31:43 +0000
Subject: 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>
```
---
 Userland/Libraries/LibWeb/HTML/Scripting/Environments.cpp | 4 +---
 Userland/Libraries/LibWeb/HTML/Scripting/Environments.h   | 2 +-
 2 files changed, 2 insertions(+), 4 deletions(-)

(limited to 'Userland/Libraries/LibWeb/HTML/Scripting')

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();
-- 
cgit v1.2.3