sfx2/source/appl/openuriexternally.cxx | 38 ++++++++++++++++++--------------- shell/source/unix/exec/shellexec.cxx | 15 ++++++++----- shell/source/win32/SysShExec.cxx | 9 ++++--- 3 files changed, 36 insertions(+), 26 deletions(-)
New commits: commit 0bbbde84b091c7969378ef9a5bbadd9706e57c5e Author: Stephan Bergmann <[email protected]> AuthorDate: Thu Jun 9 09:52:28 2022 +0200 Commit: Stephan Bergmann <[email protected]> CommitDate: Thu Jun 9 11:17:14 2022 +0200 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 <[email protected]> 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); + {}); } } }
