diff options
author | Mike Kaganski <mike.kaganski@collabora.com> | 2019-01-08 05:47:04 +0300 |
---|---|---|
committer | Thorsten Behrens <Thorsten.Behrens@CIB.de> | 2019-12-18 00:42:54 +0100 |
commit | b57678be987f9b9df39ad8d7de9a0d87bc0c6ef3 (patch) | |
tree | 182c91cf4a595b11ac37008aba7867bd8311cc48 /ucb | |
parent | bd785e8091c90df6ed5fbe51bba96929664356f6 (diff) |
Don't crash when accessing WebDAV resource after auth failed
In my testing on Windows, the crashing scenario was this:
1. FileDialogHelper_Impl::updateVersions() creates storage calling
comphelper::OStorageHelper::GetStorageFromURL;
2. Content::openStream() calls isDocument first;
3. Content::isDocument() indirectly initiates WebDAV session,
creating a NeonSession;
4. All operations of NeonSession call Init() first; its first call
initializes m_pHttpSession using ne_session_create, and then
adds auth callbacks using ne_add_server_auth/ne_add_proxy_auth
5. Then NeonSession performs the rest of PROPFIND task, calling
ah_post_send and auth_challenge; the latter fails, then
ah_post_send calls clean_session, which cleans m_pHttpSession's
auth_session's sspi_host;
6. NeonSession::HandleError throws DAVException for NE_AUTH error;
7. Content::isDocument() returns true to Content::openStream(),
which proceeds to execute the command, which in turn re-uses
the NeonSession and its m_pHttpSession;
8. NeonSession::OPTIONS then indirectly calls continue_sspi, which
tries to dereference the m_pHttpSession's auth_session's
sspi_host which is nullptr at this point.
So in case NeonSession detects the NE_AUTH error condition, let's
set a flag indicating that the next Init() should reinitialize its
m_pHttpSession.
Also fixed a case when xProps was used before initialization in
Content::getPropertyValues.
Reviewed-on: https://gerrit.libreoffice.org/65950
Tested-by: Jenkins
Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com>
(cherry picked from commit 162a472d55cf9fb9aaa6d5eae625b3da2273a516)
Conflicts:
ucb/source/ucp/webdav-neon/NeonSession.hxx
Change-Id: Ifc9eec4fe0333ff6be17c5089068441b4a6eb78c
Diffstat (limited to 'ucb')
-rw-r--r-- | ucb/source/ucp/webdav-neon/NeonSession.cxx | 14 | ||||
-rw-r--r-- | ucb/source/ucp/webdav-neon/NeonSession.hxx | 1 | ||||
-rw-r--r-- | ucb/source/ucp/webdav-neon/webdavcontent.cxx | 10 |
3 files changed, 17 insertions, 8 deletions
diff --git a/ucb/source/ucp/webdav-neon/NeonSession.cxx b/ucb/source/ucp/webdav-neon/NeonSession.cxx index aac868e3d87a..9210096dd19b 100644 --- a/ucb/source/ucp/webdav-neon/NeonSession.cxx +++ b/ucb/source/ucp/webdav-neon/NeonSession.cxx @@ -615,7 +615,8 @@ void NeonSession::Init() { osl::Guard< osl::Mutex > theGuard( m_aMutex ); - bool bCreateNewSession = false; + bool bCreateNewSession = m_bNeedNewSession; + m_bNeedNewSession = false; if ( m_pHttpSession == nullptr ) { @@ -669,13 +670,17 @@ void NeonSession::Init() m_aProxyName = rProxyCfg.aName; m_nProxyPort = rProxyCfg.nPort; + bCreateNewSession = true; + } + + if (bCreateNewSession) + { // new session needed, destroy old first { osl::Guard< osl::Mutex > theGlobalGuard( aGlobalNeonMutex ); ne_session_destroy( m_pHttpSession ); } m_pHttpSession = nullptr; - bCreateNewSession = true; } } @@ -1921,6 +1926,11 @@ void NeonSession::HandleError( int nError, m_aHostName, m_nPort ) ); case NE_AUTH: // User authentication failed on server + // m_pHttpSession could get invalidated, e.g., as result of clean_session called in + // ah_post_send in case when auth_challenge failed, which invalidates the auth_session + // which we established in Init(): the auth_session's sspi_host gets disposed, and + // next attempt to authenticate would crash in continue_sspi trying to dereference it + m_bNeedNewSession = true; throw DAVException( DAVException::DAV_HTTP_AUTH, NeonUri::makeConnectionEndPointString( m_aHostName, m_nPort ) ); diff --git a/ucb/source/ucp/webdav-neon/NeonSession.hxx b/ucb/source/ucp/webdav-neon/NeonSession.hxx index 87026ad23ec4..f66152fda6a7 100644 --- a/ucb/source/ucp/webdav-neon/NeonSession.hxx +++ b/ucb/source/ucp/webdav-neon/NeonSession.hxx @@ -54,6 +54,7 @@ private: sal_Int32 m_nProxyPort; css::uno::Sequence< css::beans::NamedValue > m_aFlags; HttpSession * m_pHttpSession; + bool m_bNeedNewSession = false; // Something happened that could invalidate m_pHttpSession void * m_pRequestData; const ucbhelper::InternetProxyDecider & m_rProxyDecider; diff --git a/ucb/source/ucp/webdav-neon/webdavcontent.cxx b/ucb/source/ucp/webdav-neon/webdavcontent.cxx index 3113b3c1f5f1..0f99aa1fe637 100644 --- a/ucb/source/ucp/webdav-neon/webdavcontent.cxx +++ b/ucb/source/ucp/webdav-neon/webdavcontent.cxx @@ -1628,12 +1628,10 @@ uno::Reference< sdbc::XRow > Content::getPropertyValues( if ( eType == DAV ) { - //xProps.reset( - // new ContentProperties( aUnescapedTitle ) ); - xProps->addProperty( - "Title", - uno::makeAny( aUnescapedTitle ), - true ); + if (!xProps) + xProps.reset(new ContentProperties(aUnescapedTitle)); + else + xProps->addProperty("Title", uno::makeAny(aUnescapedTitle), true); } else { |