Kaydet (Commit) 9ee59811 authored tarafından Mike Kaganski's avatar Mike Kaganski Kaydeden (comit) Andras Timar

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: 's avatarMike Kaganski <mike.kaganski@collabora.com>
Reviewed-on: https://gerrit.libreoffice.org/66003Tested-by: 's avatarMike Kaganski <mike.kaganski@collabora.com>
Reviewed-by: 's avatarAndras Timar <andras.timar@collabora.com>
üst 7b6cf331
...@@ -615,7 +615,8 @@ void NeonSession::Init() ...@@ -615,7 +615,8 @@ void NeonSession::Init()
{ {
osl::Guard< osl::Mutex > theGuard( m_aMutex ); osl::Guard< osl::Mutex > theGuard( m_aMutex );
bool bCreateNewSession = false; bool bCreateNewSession = m_bNeedNewSession;
m_bNeedNewSession = false;
if ( m_pHttpSession == nullptr ) if ( m_pHttpSession == nullptr )
{ {
...@@ -669,13 +670,17 @@ void NeonSession::Init() ...@@ -669,13 +670,17 @@ void NeonSession::Init()
m_aProxyName = rProxyCfg.aName; m_aProxyName = rProxyCfg.aName;
m_nProxyPort = rProxyCfg.nPort; m_nProxyPort = rProxyCfg.nPort;
bCreateNewSession = true;
}
if (bCreateNewSession)
{
// new session needed, destroy old first // new session needed, destroy old first
{ {
osl::Guard< osl::Mutex > theGlobalGuard( aGlobalNeonMutex ); osl::Guard< osl::Mutex > theGlobalGuard( aGlobalNeonMutex );
ne_session_destroy( m_pHttpSession ); ne_session_destroy( m_pHttpSession );
} }
m_pHttpSession = nullptr; m_pHttpSession = nullptr;
bCreateNewSession = true;
} }
} }
...@@ -1921,6 +1926,11 @@ void NeonSession::HandleError( int nError, ...@@ -1921,6 +1926,11 @@ void NeonSession::HandleError( int nError,
m_aHostName, m_nPort ) ); m_aHostName, m_nPort ) );
case NE_AUTH: // User authentication failed on server 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, throw DAVException( DAVException::DAV_HTTP_AUTH,
NeonUri::makeConnectionEndPointString( NeonUri::makeConnectionEndPointString(
m_aHostName, m_nPort ) ); m_aHostName, m_nPort ) );
......
...@@ -54,6 +54,7 @@ private: ...@@ -54,6 +54,7 @@ private:
sal_Int32 m_nProxyPort; sal_Int32 m_nProxyPort;
css::uno::Sequence< css::beans::NamedValue > m_aFlags; css::uno::Sequence< css::beans::NamedValue > m_aFlags;
HttpSession * m_pHttpSession; HttpSession * m_pHttpSession;
bool m_bNeedNewSession = false; // Something happened that could invalidate m_pHttpSession
void * m_pRequestData; void * m_pRequestData;
const ucbhelper::InternetProxyDecider & m_rProxyDecider; const ucbhelper::InternetProxyDecider & m_rProxyDecider;
......
...@@ -1629,12 +1629,10 @@ uno::Reference< sdbc::XRow > Content::getPropertyValues( ...@@ -1629,12 +1629,10 @@ uno::Reference< sdbc::XRow > Content::getPropertyValues(
if ( eType == DAV ) if ( eType == DAV )
{ {
//xProps.reset( if (!xProps)
// new ContentProperties( aUnescapedTitle ) ); xProps.reset(new ContentProperties(aUnescapedTitle));
xProps->addProperty( else
"Title", xProps->addProperty("Title", uno::makeAny(aUnescapedTitle), true);
uno::makeAny( aUnescapedTitle ),
true );
} }
else else
{ {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment