Clean up IsSecureURL

...to not use WildCard (in case a trusted location URI already contains an
unescaped "*"), be specific about matching only past a final "/", and rename to
isSecureMacroUri for clarification.

The check with an INET_PROT_NOT_VALID default INetURLObject in
SfxApplication::OpenDocExec_Impl ("we have to check the referer before
executing") had efficiently been dead since its inception in
14237ac4bf497decdde8b742acea23780833ba12 "#90880#: security checks corrected,"
as INET_PROT_NOT_VALID is considered secure regardless of referer anyway.

Change-Id: I03bca5e6dac89bb2aac52909aff273ea640228d8

5 files changed, 46 insertions, 99 deletions

diff --git a/include/sfx2/app.hxx b/include/sfx2/app.hxx
index d28a373bc5bf..ebb280a25ad6 100644
--- a/include/sfx2/app.hxx
+++ b/include/sfx2/app.hxx
@@ -79,7 +79,6 @@ class SfxModule;
 class SfxModule;
 typedef ::std::vector<SfxModule*> SfxModuleArr_Impl;
 class Window;
-class INetURLObject;
 struct SfxChildWinFactory;
 struct SfxMenuCtrlFactory;
 struct SfxStbCtrlFactory;
@@ -203,7 +202,6 @@ public:
     virtual void                Invalidate(sal_uInt16 nId = 0);
     void                        NotifyEvent(const SfxEventHint& rEvent, bool bSynchron = true );
     sal_Bool                        IsDowning() const;
-    sal_Bool                        IsSecureURL( const INetURLObject &rURL, const OUString *pReferer ) const;
     void                        ResetLastDir();
 
     SAL_DLLPRIVATE static SfxApplication* Get() { return pApp;}
diff --git a/include/unotools/securityoptions.hxx b/include/unotools/securityoptions.hxx
index eb6464729f8d..fc6c49c18971 100644
--- a/include/unotools/securityoptions.hxx
+++ b/include/unotools/securityoptions.hxx
@@ -181,21 +181,12 @@ class UNOTOOLS_DLLPUBLIC SAL_WARN_UNUSED SvtSecurityOptions : public utl::detail
 
         sal_Bool            IsMacroDisabled             (                   ) const ;
 
-        /*-****************************************************************************************************//**
-            @short      special method to check an URL and his referer corresponding to ouer internal security cessation
-            @descr      Give us an URL and his referer and we will say you if these url can be scripted or not!
-
-            @seealso    -
-
-            @param      "sURL" reference to URL for checking
-            @param      "sReferer" reference to referer which wish to run script by given URL
-            @return     sal_True if URL is secure or security is obsolete(!) or sal_False otherwise.
-
-            @onerror    No error should occur!
-        *//*-*****************************************************************************************************/
-
-        sal_Bool IsSecureURL(   const   OUString&    sURL        ,
-                                const   OUString&    sReferer    ) const ;
+        /**
+           Check whether the given uri is either no dangerous macro-execution
+           URI at all or else the given referer is a trusted source.
+        */
+        bool isSecureMacroUri(OUString const & uri, OUString const & referer)
+            const;
 
         ::com::sun::star::uno::Sequence< Certificate >  GetTrustedAuthors       (                                                                   ) const ;
         void                                            SetTrustedAuthors       ( const ::com::sun::star::uno::Sequence< Certificate >& rAuthors    )       ;
diff --git a/sfx2/source/appl/appcfg.cxx b/sfx2/source/appl/appcfg.cxx
index be93509b931c..18476481066a 100644
--- a/sfx2/source/appl/appcfg.cxx
+++ b/sfx2/source/appl/appcfg.cxx
@@ -494,12 +494,6 @@ sal_Bool SfxApplication::GetOptions( SfxItemSet& rSet )
     return bRet;
 }
 
-//--------------------------------------------------------------------
-sal_Bool SfxApplication::IsSecureURL( const INetURLObject& rURL, const OUString* pReferer ) const
-{
-    return SvtSecurityOptions().IsSecureURL( rURL.GetMainURL( INetURLObject::NO_DECODE ), *pReferer );
-}
-//--------------------------------------------------------------------
 // TODO/CLEANUP: Why two SetOptions Methods?
 void SfxApplication::SetOptions_Impl( const SfxItemSet& rSet )
 {
diff --git a/sfx2/source/appl/appopen.cxx b/sfx2/source/appl/appopen.cxx
index 111257331899..e37692935b5e 100644
--- a/sfx2/source/appl/appopen.cxx
+++ b/sfx2/source/appl/appopen.cxx
@@ -931,28 +931,17 @@ void SfxApplication::OpenDocExec_Impl( SfxRequest& rReq )
                     if ( !bFound )
                     {
                         sal_Bool bLoadInternal = sal_False;
-
-                        // security reservation: => we have to check the referer before executing
-                        if (SFX_APP()->IsSecureURL(INetURLObject(), &aReferer))
+                        try
                         {
-                            try
-                            {
-                                sfx2::openUriExternally(
-                                    aURL.Complete, pFilter == 0);
-                            }
-                            catch ( ::com::sun::star::system::SystemShellExecuteException& )
-                            {
-                                rReq.RemoveItem( SID_TARGETNAME );
-                                rReq.AppendItem( SfxStringItem( SID_TARGETNAME, OUString("_default") ) );
-                                bLoadInternal = sal_True;
-                            }
+                            sfx2::openUriExternally(
+                                aURL.Complete, pFilter == 0);
                         }
-                        else
+                        catch ( ::com::sun::star::system::SystemShellExecuteException& )
                         {
-                            SfxErrorContext aCtx( ERRCTX_SFX_OPENDOC, aURL.Complete );
-                            ErrorHandler::HandleError( ERRCODE_IO_ACCESSDENIED );
+                            rReq.RemoveItem( SID_TARGETNAME );
+                            rReq.AppendItem( SfxStringItem( SID_TARGETNAME, OUString("_default") ) );
+                            bLoadInternal = sal_True;
                         }
-
                         if ( !bLoadInternal )
                             return;
                     }
@@ -967,7 +956,7 @@ void SfxApplication::OpenDocExec_Impl( SfxRequest& rReq )
         }
     }
 
-    if ( !SFX_APP()->IsSecureURL( INetURLObject(aFileName), &aReferer ) )
+    if (!SvtSecurityOptions().isSecureMacroUri(aFileName, aReferer))
     {
         SfxErrorContext aCtx( ERRCTX_SFX_OPENDOC, aFileName );
         ErrorHandler::HandleError( ERRCODE_IO_ACCESSDENIED );
diff --git a/unotools/source/config/securityoptions.cxx b/unotools/source/config/securityoptions.cxx
index efcb2a1dcd7f..2271219747f9 100644
--- a/unotools/source/config/securityoptions.cxx
+++ b/unotools/source/config/securityoptions.cxx
@@ -28,7 +28,6 @@
 #include <com/sun/star/beans/PropertyValue.hpp>
 #include <comphelper/sequenceasvector.hxx>
 #include <tools/urlobj.hxx>
-#include <tools/wldcrd.hxx>
 
 #include <unotools/pathoptions.hxx>
 
@@ -175,8 +174,6 @@ class SvtSecurityOptions_Impl : public ConfigItem
 
         Sequence< OUString >    GetSecureURLs   (                                                       ) const ;
         void                    SetSecureURLs   (   const   Sequence< OUString >&   seqURLList          )       ;
-        sal_Bool                IsSecureURL     (   const   OUString&               sURL,
-                                                    const   OUString&               sReferer            ) const ;
         inline sal_Int32        GetMacroSecurityLevel   (                                               ) const ;
         void                    SetMacroSecurityLevel   ( sal_Int32 _nLevel                             )       ;
 
@@ -188,7 +185,6 @@ class SvtSecurityOptions_Impl : public ConfigItem
         sal_Bool                IsOptionSet     ( SvtSecurityOptions::EOption eOption                   ) const ;
         sal_Bool                SetOption       ( SvtSecurityOptions::EOption eOption, sal_Bool bValue  )       ;
         sal_Bool                IsOptionEnabled ( SvtSecurityOptions::EOption eOption                   ) const ;
-private:
 
         /*-****************************************************************************************************//**
             @short      return list of key names of ouer configuration management which represent our module tree
@@ -864,55 +860,6 @@ void SvtSecurityOptions_Impl::SetSecureURLs( const Sequence< OUString >& seqURLL
     }
 }
 
-sal_Bool SvtSecurityOptions_Impl::IsSecureURL(  const   OUString&   sURL    ,
-                                                const   OUString&   sReferer) const
-{
-    sal_Bool bState = sal_False;
-
-    // Check for uncritical protocols first
-    // All protocols different from "macro..." and "slot..." are secure per definition and must not be checked.
-    // "macro://#..." means AppBasic macros that are considered safe
-    INetURLObject   aURL        ( sURL );
-    INetProtocol    aProtocol   = aURL.GetProtocol();
-
-    // All other URLs must checked in combination with referer and internal information about security
-    if ( (aProtocol != INET_PROT_MACRO && aProtocol !=  INET_PROT_SLOT) ||
-         aURL.GetMainURL( INetURLObject::NO_DECODE ).matchIgnoreAsciiCaseAsciiL( "macro:///", 9 ) == 0)
-    {
-        // security check only for "macro" ( without app basic ) or "slot" protocols
-        bState = sal_True;
-    }
-    else
-    {
-        //  check list of allowed URL patterns
-        // Trusted referer given?
-        // NO  => bState will be false per default
-        // YES => search for it in our internal url list
-        if( !sReferer.isEmpty() )
-        {
-            // Search in internal list
-            OUString sRef = sReferer.toAsciiLowerCase();
-            sal_uInt32 nCount = m_seqSecureURLs.getLength();
-            for( sal_uInt32 nItem=0; nItem<nCount; ++nItem )
-            {
-                OUString sCheckURL = m_seqSecureURLs[nItem].toAsciiLowerCase();
-                sCheckURL += "*";
-                if( WildCard( sCheckURL ).Matches( sRef ) == sal_True )
-                {
-                    bState = sal_True;
-                    break;
-                }
-            }
-
-            if ( !bState )
-                bState = sRef.compareToAscii("private:user") == 0;
-        }
-    }
-
-    // Return result of operation.
-    return bState;
-}
-
 inline sal_Int32 SvtSecurityOptions_Impl::GetMacroSecurityLevel() const
 {
     return m_nSecLevel;
@@ -1082,11 +1029,39 @@ void SvtSecurityOptions::SetSecureURLs( const Sequence< OUString >& seqURLList )
     m_pDataContainer->SetSecureURLs( seqURLList );
 }
 
-sal_Bool SvtSecurityOptions::IsSecureURL(   const   OUString&   sURL        ,
-                                            const   OUString&   sReferer    ) const
+bool SvtSecurityOptions::isSecureMacroUri(
+    OUString const & uri, OUString const & referer) const
 {
-    MutexGuard aGuard( GetInitMutex() );
-    return m_pDataContainer->IsSecureURL( sURL, sReferer );
+    switch (INetURLObject(uri).GetProtocol()) {
+    case INET_PROT_MACRO:
+        if (uri.startsWithIgnoreAsciiCase("macro:///")) {
+            // Denotes an App-BASIC macro (see SfxMacroLoader::loadMacro), which
+            // is considered safe:
+            return true;
+        }
+        // fall through
+    case INET_PROT_SLOT:
+        if (referer.equalsIgnoreAsciiCase("private:user")) {
+            return true;
+        }
+        {
+            MutexGuard g(GetInitMutex());
+            for (sal_Int32 i = 0;
+                 i != m_pDataContainer->m_seqSecureURLs.getLength(); ++i)
+            {
+                OUString pref(m_pDataContainer->m_seqSecureURLs[i]);
+                pref.endsWith("/", &pref);
+                if (referer.equalsIgnoreAsciiCase(pref)
+                    || referer.startsWithIgnoreAsciiCase(pref + "/"))
+                {
+                    return true;
+                }
+            }
+            return false;
+        }
+    default:
+        return true;
+    }
 }
 
 sal_Int32 SvtSecurityOptions::GetMacroSecurityLevel() const