ucb/source/ucp/webdav-neon/NeonSession.cxx | 14 ++++++++++++-- ucb/source/ucp/webdav-neon/NeonSession.hxx | 1 + ucb/source/ucp/webdav-neon/webdavcontent.cxx | 10 ++++------ 3 files changed, 17 insertions(+), 8 deletions(-)
New commits: commit 9ee5981193db8d35a80d1cba075895e226ff95e8 Author: Mike Kaganski <mike.kagan...@collabora.com> AuthorDate: Tue Jan 8 05:47:04 2019 +0300 Commit: Andras Timar <andras.ti...@collabora.com> CommitDate: Fri Jan 11 23:18:05 2019 +0100 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. Change-Id: Ifc9eec4fe0333ff6be17c5089068441b4a6eb78c Reviewed-on: https://gerrit.libreoffice.org/65950 Tested-by: Jenkins Reviewed-by: Mike Kaganski <mike.kagan...@collabora.com> Reviewed-on: https://gerrit.libreoffice.org/66003 Tested-by: Mike Kaganski <mike.kagan...@collabora.com> Reviewed-by: Andras Timar <andras.ti...@collabora.com> diff --git a/ucb/source/ucp/webdav-neon/NeonSession.cxx b/ucb/source/ucp/webdav-neon/NeonSession.cxx index 3af9a6aba60f..ba87fceb551f 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 db20772a62f2..1246328edaee 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 c49caa3d8367..9d2a689b2270 100644 --- a/ucb/source/ucp/webdav-neon/webdavcontent.cxx +++ b/ucb/source/ucp/webdav-neon/webdavcontent.cxx @@ -1629,12 +1629,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 { _______________________________________________ Libreoffice-commits mailing list libreoffice-comm...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits