tdf#140886: Make "Do you really want to open it?" more reliable

70009098fd70df021048c540d1796c928554b494 "tdf#128969: Let the user explicitly
decide to execute an external program" had shoehorned that new warning dialog
into the existing XSystemShellExecute::execute IllegalArgumentException return
path, which caused some issues:  For example, it caused the warning dialog to
reappear after you acknowledged it on macOS (see comment at
<https://bugs.documentfoundation.org/show_bug.cgi?id=140886#c10> "Allow
hyperlink opening on file with execute bit set ref. CVE-2019-9847"), and it
caused the warning dialog to erroneously appear for a non-existing file on
Windows (see comment at
<https://gerrit.libreoffice.org/c/core/+/124422/2#message-ac76b728fedc53e7d0a04c99f00364068b51a8ea>
"tdf#128969: Let the user explicitly decide to execute an external program").

So rather than reusing IllegalArgumentException for this case, use a different
kind of exception to trigger that warning dialog.  The existing
AccessControlException (which is also a RuntimeException) happened to fit more
or less well.

Change-Id: I3f743c21be48d54f10951006ef3d7172e23e9076
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/135524
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>

3 files changed, 36 insertions, 26 deletions

diff --git a/sfx2/source/appl/openuriexternally.cxx b/sfx2/source/appl/openuriexternally.cxx
index 7cc923aba241..a8aed34fcfdf 100644
--- a/sfx2/source/appl/openuriexternally.cxx
+++ b/sfx2/source/appl/openuriexternally.cxx
@@ -10,6 +10,7 @@
 #include <sal/config.h>
 
 #include <com/sun/star/lang/IllegalArgumentException.hpp>
+#include <com/sun/star/security/AccessControlException.hpp>
 #include <com/sun/star/system/SystemShellExecute.hpp>
 #include <com/sun/star/system/SystemShellExecuteException.hpp>
 #include <com/sun/star/system/SystemShellExecuteFlags.hpp>
@@ -85,29 +86,32 @@ IMPL_LINK_NOARG(URITools, onOpenURI, Timer*, void)
     for (sal_Int32 flags = css::system::SystemShellExecuteFlags::URIS_ONLY;;) {
         try {
             exec->execute(msURI, OUString(), flags);
+        } catch (css::security::AccessControlException & e) {
+            if (e.LackingPermission.hasValue() || flags == 0) {
+                throw css::uno::RuntimeException(
+                    "unexpected AccessControlException: " + e.Message);
+            }
+            SolarMutexGuard g;
+            std::unique_ptr<weld::MessageDialog> eb(
+                Application::CreateMessageDialog(
+                    mpDialogParent, VclMessageType::Warning, VclButtonsType::OkCancel,
+                    SfxResId(STR_DANGEROUS_TO_OPEN)));
+            eb->set_primary_text(eb->get_primary_text().replaceFirst("$(ARG1)", INetURLObject::decode(msURI, INetURLObject::DecodeMechanism::Unambiguous)));
+            if (eb->run() == RET_OK) {
+                flags = 0;
+                continue;
+            }
         } catch (css::lang::IllegalArgumentException & e) {
             if (e.ArgumentPosition != 0) {
                 throw css::uno::RuntimeException(
                     "unexpected IllegalArgumentException: " + e.Message);
             }
             SolarMutexGuard g;
-            if (flags == css::system::SystemShellExecuteFlags::URIS_ONLY) {
-                std::unique_ptr<weld::MessageDialog> eb(
-                    Application::CreateMessageDialog(
-                        mpDialogParent, VclMessageType::Warning, VclButtonsType::OkCancel,
-                        SfxResId(STR_DANGEROUS_TO_OPEN)));
-                eb->set_primary_text(eb->get_primary_text().replaceFirst("$(ARG1)", INetURLObject::decode(msURI, INetURLObject::DecodeMechanism::Unambiguous)));
-                if (eb->run() == RET_OK) {
-                    flags = 0;
-                    continue;
-                }
-            } else {
-                std::unique_ptr<weld::MessageDialog> eb(Application::CreateMessageDialog(mpDialogParent,
-                                                                         VclMessageType::Warning, VclButtonsType::Ok,
-                                                                         SfxResId(STR_NO_ABS_URI_REF)));
-                eb->set_primary_text(eb->get_primary_text().replaceFirst("$(ARG1)", INetURLObject::decode(msURI, INetURLObject::DecodeMechanism::Unambiguous)));
-                eb->run();
-            }
+            std::unique_ptr<weld::MessageDialog> eb(Application::CreateMessageDialog(mpDialogParent,
+                                                                     VclMessageType::Warning, VclButtonsType::Ok,
+                                                                     SfxResId(STR_NO_ABS_URI_REF)));
+            eb->set_primary_text(eb->get_primary_text().replaceFirst("$(ARG1)", INetURLObject::decode(msURI, INetURLObject::DecodeMechanism::Unambiguous)));
+            eb->run();
         } catch (css::system::SystemShellExecuteException & e) {
             if (!mbHandleSystemShellExecuteException) {
                 throw;
diff --git a/shell/source/unix/exec/shellexec.cxx b/shell/source/unix/exec/shellexec.cxx
index 240113ae8271..195a697888c2 100644
--- a/shell/source/unix/exec/shellexec.cxx
+++ b/shell/source/unix/exec/shellexec.cxx
@@ -27,6 +27,7 @@
 #include <com/sun/star/system/SystemShellExecuteFlags.hpp>
 
 #include <com/sun/star/lang/IllegalArgumentException.hpp>
+#include <com/sun/star/security/AccessControlException.hpp>
 #include <com/sun/star/uri/ExternalUriReferenceTranslator.hpp>
 #include <com/sun/star/uri/UriReferenceFactory.hpp>
 #include <cppuhelper/supportsservice.hxx>
@@ -136,13 +137,17 @@ void SAL_CALL ShellExec::execute( const OUString& aCommand, const OUString& aPar
                 auto const e3 = errno;
                 SAL_INFO("shell", "lstat(" << pathname8 << ") failed with errno " << e3);
             }
-            if (e2 == 0 && (S_ISDIR(st.st_mode) || S_ISLNK(st.st_mode))) {
-                dir = true;
-            } else if (e2 != 0 || !S_ISREG(st.st_mode)
-                       || (st.st_mode & (S_IXUSR | S_IXGRP | S_IXOTH)) != 0)
-            {
+            if (e2 != 0) {
                 throw css::lang::IllegalArgumentException(
                     "XSystemShellExecute.execute, cannot process <" + aCommand + ">", {}, 0);
+            } else if (S_ISDIR(st.st_mode) || S_ISLNK(st.st_mode)) {
+                dir = true;
+            } else if ((nFlags & css::system::SystemShellExecuteFlags::URIS_ONLY) != 0
+                       && (!S_ISREG(st.st_mode)
+                           || (st.st_mode & (S_IXUSR | S_IXGRP | S_IXOTH)) != 0))
+            {
+                throw css::security::AccessControlException(
+                    "XSystemShellExecute.execute, bad <" + aCommand + ">", {}, {});
             } else if (pathname.endsWithIgnoreAsciiCase(".class")
                        || pathname.endsWithIgnoreAsciiCase(".dmg")
                        || pathname.endsWithIgnoreAsciiCase(".fileloc")
diff --git a/shell/source/win32/SysShExec.cxx b/shell/source/win32/SysShExec.cxx
index 066b818abe92..7be77d6344de 100644
--- a/shell/source/win32/SysShExec.cxx
+++ b/shell/source/win32/SysShExec.cxx
@@ -29,6 +29,7 @@
 #include <osl/file.hxx>
 #include <sal/macros.h>
 #include <com/sun/star/lang/IllegalArgumentException.hpp>
+#include <com/sun/star/security/AccessControlException.hpp>
 #include <com/sun/star/system/SystemShellExecuteException.hpp>
 #include <com/sun/star/system/SystemShellExecuteFlags.hpp>
 #include <com/sun/star/uri/UriReferenceFactory.hpp>
@@ -268,8 +269,8 @@ void SAL_CALL CSysShExec::execute( const OUString& aCommand, const OUString& aPa
                 SHFILEINFOW info;
                 if (SHGetFileInfoW(path, 0, &info, sizeof info, SHGFI_EXETYPE) != 0)
                 {
-                    throw css::lang::IllegalArgumentException(
-                        "XSystemShellExecute.execute, cannot process <" + aCommand + ">", {}, 0);
+                    throw css::security::AccessControlException(
+                        "XSystemShellExecute.execute, cannot process <" + aCommand + ">", {}, {});
                 }
                 if (SHGetFileInfoW(path, 0, &info, sizeof info, SHGFI_ATTRIBUTES) == 0)
                 {
@@ -353,9 +354,9 @@ void SAL_CALL CSysShExec::execute( const OUString& aCommand, const OUString& aPa
                               ".PS1XML;.PS2;.PS2XML;.PSC1;.PSC2;.PY;.REG;.SCF;.SCR;.SCT;.SHB;.SYS;"
                               ".VB;.VBE;.VBS;.VXD;.WS;.WSC;.WSF;.WSH;")))
                     {
-                        throw css::lang::IllegalArgumentException(
+                        throw css::security::AccessControlException(
                             "XSystemShellExecute.execute, cannot process <" + aCommand + ">", {},
-                            0);
+                            {});
                     }
                 }
             }