[Libreoffice-commits] core.git: ucb/source

[Libreoffice Gerrit user]

 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 162a472d55cf9fb9aaa6d5eae625b3da2273a516
Author:     Mike Kaganski <mike.kaganski at collabora.com>
AuthorDate: Tue Jan 8 05:47:04 2019 +0300
Commit:     Mike Kaganski <mike.kaganski at collabora.com>
CommitDate: Tue Jan 8 06:34:55 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.kaganski at collabora.com>

diff --git a/ucb/source/ucp/webdav-neon/NeonSession.cxx b/ucb/source/ucp/webdav-neon/NeonSession.cxx
index 979fb3819406..6d1215b7fe21 100644
--- a/ucb/source/ucp/webdav-neon/NeonSession.cxx
+++ b/ucb/source/ucp/webdav-neon/NeonSession.cxx
@@ -671,7 +671,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 )
     {
@@ -725,13 +726,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(getGlobalNeonMutex());
                 ne_session_destroy( m_pHttpSession );
             }
             m_pHttpSession = nullptr;
-            bCreateNewSession = true;
         }
     }
 
@@ -1966,6 +1971,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 2adebaacd3fd..df4522da6e18 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 > const m_aFlags;
     HttpSession *     m_pHttpSession;
+    bool m_bNeedNewSession = false; // Something happened that could invalidate m_pHttpSession
     void * const      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 7a376f48fcfa..8ef0956b2eff 100644
--- a/ucb/source/ucp/webdav-neon/webdavcontent.cxx
+++ b/ucb/source/ucp/webdav-neon/webdavcontent.cxx
@@ -1586,12 +1586,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
         {