From c4e2c725ecedbe5442411b5590a3b2d73031b847 Mon Sep 17 00:00:00 2001 From: Timothy Flynn Date: Fri, 29 Dec 2023 09:05:05 -0500 Subject: [PATCH] Ladybird/Qt: Ensure the Inspector widget is deleted before the WebView According to Qt documentation, destruction of a QObject's children may happen in any order. In our case, the Tab's WebContentView is deleted before its InspectorWidget. The InspectorWidget performs cleanup on that WebContentView in its destructor; this causes use-after-free since it has already been destroyed. From reading Qt threads, if a particular destruction order is required, it is okay to enforce that order with manual `delete`s. This patch does so with the InspectorWidget to ensure it is deleted before the WebContentView. Qt's object ownership is okay with this - it will remove the InspectorWidget from the Tab's children, preventing any double deletion. --- Ladybird/Qt/Tab.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Ladybird/Qt/Tab.cpp b/Ladybird/Qt/Tab.cpp index 7ae21f96ab..bc3f7c8535 100644 --- a/Ladybird/Qt/Tab.cpp +++ b/Ladybird/Qt/Tab.cpp @@ -579,6 +579,10 @@ Tab::Tab(BrowserWindow* window, WebContentOptions const& web_content_options, St Tab::~Tab() { close_sub_widgets(); + + // Delete the InspectorWidget explicitly to ensure it is deleted before the WebContentView. Otherwise, Qt + // can destroy these objects in any order, which may cause use-after-free in InspectorWidget's destructor. + delete m_inspector_widget; } void Tab::select_dropdown_add_item(QMenu* menu, Web::HTML::SelectItem const& item)