use after free on clicking cancel in a calc spelling message dialog

e.g. the calc subdialog "Should the spellcheck be continued from the
beginning of the current sheet" seen from the "spelling" dialog when
starting spelling from some cell beyond used sheet bounds.

With a local asan build, and hello-world.ods, load, put cursor in A2,
review, spelling to get the spelling dialog. There should then be a sub
dialog with "Should the spellcheck be continued ..." with "No" and "Yes"
options, hammer the "no" button a few times in rapid succession.

2715237==ERROR: AddressSanitizer: heap-use-after-free on address 0x6140007a8118 at pc 0x7fdf28e73ce1 bp 0x7ffd012c88d0 sp 0x7ffd012c88c8
READ of size 8 at 0x6140007a8118 thread T0 (kitbroker_003)
#0 0x7fdf28e73ce0  (instdir/program/libvcllo.so+0x2473ce0)

JSMessageDialog::~JSMessageDialog has:

JSMessageDialog::~JSMessageDialog()
{
if (m_pOK || m_pCancel)
JSInstanceBuilder::RemoveWindowWidget(m_sWindowId);
}

but has

int JSMessageDialog::run()
{
if (GetLOKNotifier())
{
RememberMessageDialog();
sendFullUpdate();
}
...
}

where RememberMessageDialog has JSInstanceBuilder::InsertWindowToMap(sWindowId);

this dialog doesn't have ok or cancel, so while it is inserted in that
map when the dialog is run, it doesn't get removed from the map when the
dtor is called, which goes on to cause use-after-free.

Given that we only set m_pCancel and/or m_pOK if there is no "builder"
it looks a more straight forward approach to simply call
JSInstanceBuilder::RemoveWindowWidget based on if there was no builder.

Change-Id: Icf04f0e9f3c3c864955e9d4ee41589f4d2aa4cb3
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/163623
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice@gmail.com>
Reviewed-by: Caolán McNamara <caolan.mcnamara@collabora.com>

1 files changed, 6 insertions, 1 deletions

diff --git a/vcl/jsdialog/jsdialogbuilder.cxx b/vcl/jsdialog/jsdialogbuilder.cxx
index 904ee9076f79..f5e3a9c65325 100644
--- a/vcl/jsdialog/jsdialogbuilder.cxx
+++ b/vcl/jsdialog/jsdialogbuilder.cxx
@@ -1820,8 +1820,13 @@ JSMessageDialog::JSMessageDialog(::MessageDialog* pDialog, SalInstanceBuilder* p
 
 JSMessageDialog::~JSMessageDialog()
 {
-    if (m_pOK || m_pCancel)
+    if (!m_pBuilder)
+    {
+        // For Message Dialogs created from Application::CreateMessageDialog
+        // (where there is no builder to take care of this for us) explicitly
+        // remove this window id on tear down
         JSInstanceBuilder::RemoveWindowWidget(m_sWindowId);
+    }
 }
 
 void JSMessageDialog::RememberMessageDialog()