Don't reuse CertificateChooser instances

Reusing the same instance will, in the following case, lead to a
crash. It appears that the CertificateChooser is getting disposed
somewhere as mpDialogImpl in its base class ends up being null:

1. Create an empty Writer document and add a digital signature
   in the Digital Signatures dialog
2. File > Save As the document, check the "Encrypt with GPG key"
   checkbox, press Encrypt, and crash in Dialog::ImplStartExecute()

Change-Id: I9aaa1bd449622e018b502d68c53d397255a1b61a
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/163065
Tested-by: Jenkins
Reviewed-by: Caolán McNamara <caolan.mcnamara@collabora.com>
Reviewed-by: Patrick Luby <guibomacdev@gmail.com>

3 files changed, 12 insertions, 10 deletions

diff --git a/xmlsecurity/inc/certificatechooser.hxx b/xmlsecurity/inc/certificatechooser.hxx
index 12303fbf1cd5..b0cf7c7cdcc4 100644
--- a/xmlsecurity/inc/certificatechooser.hxx
+++ b/xmlsecurity/inc/certificatechooser.hxx
@@ -51,8 +51,6 @@ enum class UserAction
 class CertificateChooser final : public weld::GenericDialogController
 {
 private:
-    static inline CertificateChooser* mxInstance = nullptr;
-
     std::vector< css::uno::Reference< css::xml::crypto::XXMLSecurityContext > > mxSecurityContexts;
     std::vector<std::shared_ptr<UserData>> mvUserData;
 
@@ -91,14 +89,18 @@ public:
                        UserAction eAction);
     virtual ~CertificateChooser() override;
 
-    static CertificateChooser* getInstance(weld::Window* _pParent,
+    static std::unique_ptr<CertificateChooser> getInstance(weld::Window* _pParent,
                         std::vector< css::uno::Reference< css::xml::crypto::XXMLSecurityContext > > && rxSecurityContexts,
                         UserAction eAction) {
-        if (!mxInstance)
-        {
-            mxInstance = new CertificateChooser(_pParent, std::move(rxSecurityContexts), eAction);
-        }
-        return mxInstance;
+        // Don't reuse CertificateChooser instances
+        // Reusing the same instance will, in the following case, lead to a
+        // crash. It appears that the CertificateChooser is getting disposed
+        // somewhere as mpDialogImpl in its base class ends up being null:
+        // 1. Create an empty Writer document and add a digital signature
+        //    in the Digital Signatures dialog
+        // 2. File > Save As the document, check the "Encrypt with GPG key"
+        //    checkbox, press Encrypt, and crash in Dialog::ImplStartExecute()
+        return std::make_unique<CertificateChooser>(_pParent, std::move(rxSecurityContexts), eAction);
     }
 
     short run() override;
diff --git a/xmlsecurity/source/component/documentdigitalsignatures.cxx b/xmlsecurity/source/component/documentdigitalsignatures.cxx
index 4ad63b36ed0b..c1768c0e953a 100644
--- a/xmlsecurity/source/component/documentdigitalsignatures.cxx
+++ b/xmlsecurity/source/component/documentdigitalsignatures.cxx
@@ -709,7 +709,7 @@ DocumentDigitalSignatures::chooseCertificatesImpl(std::map<OUString, OUString>&
             xSecContexts.push_back(aSignatureManager.getGpgSecurityContext());
     }
 
-    CertificateChooser* aChooser = CertificateChooser::getInstance(Application::GetFrameWeld(mxParentWindow), std::move(xSecContexts), eAction);
+    std::unique_ptr<CertificateChooser> aChooser = CertificateChooser::getInstance(Application::GetFrameWeld(mxParentWindow), std::move(xSecContexts), eAction);
 
     if (aChooser->run() != RET_OK)
         return { Reference< css::security::XCertificate >(nullptr) };
diff --git a/xmlsecurity/source/dialogs/digitalsignaturesdialog.cxx b/xmlsecurity/source/dialogs/digitalsignaturesdialog.cxx
index b1a2cd57c95e..8349a58a31ce 100644
--- a/xmlsecurity/source/dialogs/digitalsignaturesdialog.cxx
+++ b/xmlsecurity/source/dialogs/digitalsignaturesdialog.cxx
@@ -509,7 +509,7 @@ IMPL_LINK_NOARG(DigitalSignaturesDialog, AddButtonHdl, weld::Button&, void)
         if (DocumentSignatureHelper::CanSignWithGPG(maSignatureManager.getStore(), m_sODFVersion))
             xSecContexts.push_back(maSignatureManager.getGpgSecurityContext());
 
-        CertificateChooser* aChooser = CertificateChooser::getInstance(m_xDialog.get(), std::move(xSecContexts), UserAction::Sign);
+        std::unique_ptr<CertificateChooser> aChooser = CertificateChooser::getInstance(m_xDialog.get(), std::move(xSecContexts), UserAction::Sign);
         if (aChooser->run() == RET_OK)
         {
             sal_Int32 nSecurityId;