From 48e11a1f128795092d986499ed33255ad9b30afe Mon Sep 17 00:00:00 2001
From: Luke Wilde <lukew@serenityos.org>
Date: Fri, 1 Mar 2024 00:58:51 +0000
Subject: [PATCH] LibWeb: Empty CE reaction queue instead of destroying it on
 exception

If an exception occurs in a custom element constructor, we clear the
reaction queue by destroying it, instead of emptying the Vector.
https://github.com/SerenityOS/serenity/blob/3da6916383afc1c8a8b59dae38c4bc02680b50ba/Userland/Libraries/LibWeb/DOM/Element.cpp#L2033

This causes a UAF here, as async upgrades (i.e. custom elements not
created by document.createElement) are performed in this loop:
https://github.com/SerenityOS/serenity/blob/3da6916383afc1c8a8b59dae38c4bc02680b50ba/Userland/Libraries/LibWeb/Bindings/MainThreadVM.cpp#L657

Fixes crash when loading https://github.com/SerenityOS/serenity
---
 .../custom-elements-throw-in-constructor.txt  |  2 ++
 .../custom-elements-throw-in-constructor.html | 20 +++++++++++++++++++
 Userland/Libraries/LibWeb/DOM/Element.cpp     |  3 ++-
 3 files changed, 24 insertions(+), 1 deletion(-)
 create mode 100644 Tests/LibWeb/Text/expected/HTML/custom-elements-throw-in-constructor.txt
 create mode 100644 Tests/LibWeb/Text/input/HTML/custom-elements-throw-in-constructor.html

diff --git a/Tests/LibWeb/Text/expected/HTML/custom-elements-throw-in-constructor.txt b/Tests/LibWeb/Text/expected/HTML/custom-elements-throw-in-constructor.txt
new file mode 100644
index 0000000000..6d6aa3c071
--- /dev/null
+++ b/Tests/LibWeb/Text/expected/HTML/custom-elements-throw-in-constructor.txt
@@ -0,0 +1,2 @@
+   Entered TestElement constructor, throwing.
+PASS! (Didn't crash)
diff --git a/Tests/LibWeb/Text/input/HTML/custom-elements-throw-in-constructor.html b/Tests/LibWeb/Text/input/HTML/custom-elements-throw-in-constructor.html
new file mode 100644
index 0000000000..39481e5093
--- /dev/null
+++ b/Tests/LibWeb/Text/input/HTML/custom-elements-throw-in-constructor.html
@@ -0,0 +1,20 @@
+<test-element></test-element>
+<script src="../include.js"></script>
+<script>
+    test(() => {
+        class TestElement extends HTMLElement {
+            constructor() {
+                super();
+                println("Entered TestElement constructor, throwing.");
+                throw "test";
+            }
+
+            connectedCallback() {
+                println("connectedCallback");
+            }
+        }
+
+        customElements.define("test-element", TestElement);
+        println("PASS! (Didn't crash)");
+    });
+</script>
diff --git a/Userland/Libraries/LibWeb/DOM/Element.cpp b/Userland/Libraries/LibWeb/DOM/Element.cpp
index b9306a3c75..886bf5e044 100644
--- a/Userland/Libraries/LibWeb/DOM/Element.cpp
+++ b/Userland/Libraries/LibWeb/DOM/Element.cpp
@@ -2030,7 +2030,8 @@ JS::ThrowCompletionOr<void> Element::upgrade_element(JS::NonnullGCPtr<HTML::Cust
         m_custom_element_definition = nullptr;
 
         // 2. Empty element's custom element reaction queue.
-        m_custom_element_reaction_queue = nullptr;
+        if (m_custom_element_reaction_queue)
+            m_custom_element_reaction_queue->clear();
 
         // 3. Rethrow the exception (thus terminating this algorithm).
         return maybe_exception.release_error();