vcl/jsdialog/jsdialogbuilder.cxx | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
New commits:
commit 03136175df2ce6b82e7b814ba2308aba3d4915a8
Author: Caolán McNamara <caolan.mcnam...@collabora.com>
AuthorDate: Wed Feb 21 12:30:41 2024 +0000
Commit: Caolán McNamara <caolan.mcnam...@collabora.com>
CommitDate: Wed Feb 21 14:31:42 2024 +0100
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/+/163691
Reviewed-by: Szymon Kłos <szymon.k...@collabora.com>
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoff...@gmail.com>
diff --git a/vcl/jsdialog/jsdialogbuilder.cxx b/vcl/jsdialog/jsdialogbuilder.cxx
index c2cea61a6197..a55c7b9fbf5b 100644
--- a/vcl/jsdialog/jsdialogbuilder.cxx
+++ b/vcl/jsdialog/jsdialogbuilder.cxx
@@ -1836,8 +1836,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()
