diff options
author | Stephan Bergmann <sbergman@redhat.com> | 2013-11-08 17:25:45 +0100 |
---|---|---|
committer | Stephan Bergmann <sbergman@redhat.com> | 2013-11-08 17:34:44 +0100 |
commit | ea1a7ba72e1bd50a12faff1f8180a5a44745715d (patch) | |
tree | 2c59c7de5cce89a42338836d718581da0bd846a0 | |
parent | 81eba5c49dae5ba9efcdc8632044dc853afbf7b6 (diff) |
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
-rw-r--r-- | include/sfx2/app.hxx | 2 | ||||
-rw-r--r-- | include/unotools/securityoptions.hxx | 21 | ||||
-rw-r--r-- | sfx2/source/appl/appcfg.cxx | 6 | ||||
-rw-r--r-- | sfx2/source/appl/appopen.cxx | 27 | ||||
-rw-r--r-- | unotools/source/config/securityoptions.cxx | 89 |
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 |