vcl/jsdialog/jsdialogbuilder.cxx | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
New commits: commit 6b214f90bf6c66a7d2ed9cfb45ae8174e508fc45 Author: Caolán McNamara <[email protected]> AuthorDate: Wed Feb 21 12:30:41 2024 +0000 Commit: Caolán McNamara <[email protected]> CommitDate: Wed Feb 21 16:18:30 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/+/163624 Tested-by: Jenkins Reviewed-by: Caolán McNamara <[email protected]> diff --git a/vcl/jsdialog/jsdialogbuilder.cxx b/vcl/jsdialog/jsdialogbuilder.cxx index a65e66f96cea..abd57bf32b85 100644 --- a/vcl/jsdialog/jsdialogbuilder.cxx +++ b/vcl/jsdialog/jsdialogbuilder.cxx @@ -1829,8 +1829,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()
