crashtesting: UaF on layout of fdo53985-1.docx

Reviewed-on: https://gerrit.libreoffice.org/c/core/+/119060
Tested-by: Jenkins
Reviewed-by: Caolán McNamara <caolanm@redhat.com>
(cherry picked from commit ceb32f59d96a17c3007ed883fb44bc880673c8e0)

Change-Id: Id8ca0d277f485347e21bd8d6d68de2a7de13de48

2 files changed, 58 insertions, 1 deletions

diff --git a/sw/source/core/inc/layact.hxx b/sw/source/core/inc/layact.hxx
index 3e6ded269bd4..5a00382995f9 100644
--- a/sw/source/core/inc/layact.hxx
+++ b/sw/source/core/inc/layact.hxx
@@ -68,6 +68,9 @@ class SwLayAction
 
     std::unique_ptr<SwWait> m_pWait;
 
+    std::vector<SwFrame*> m_aFrameStack;
+    std::vector<std::unique_ptr<SwFrameDeleteGuard>> m_aFrameDeleteGuards;
+
     // If a paragraph (or anything else) moved more than one page when
     // formatting, it adds its new page number here.
     // The InternalAction can then take the appropriate steps.
@@ -124,6 +127,9 @@ class SwLayAction
 
     bool RemoveEmptyBrowserPages();
 
+    void PushFormatLayout(SwFrame* pLow);
+    void PopFormatLayout();
+
 public:
     SwLayAction(SwRootFrame *pRt, SwViewShellImp *pImp, TaskStopwatch* pWatch = nullptr);
     ~SwLayAction();
@@ -148,7 +154,7 @@ public:
     void SetReschedule  ( bool bNew )   { m_bReschedule = bNew; }
     void SetWaitAllowed ( bool bNew )   { m_bWaitAllowed = bNew; }
 
-    void SetAgain(bool bAgain) { m_bAgain = bAgain; }
+    void SetAgain(bool bAgain);
     void SetUpdateExpFields() {m_bUpdateExpFields = true; }
 
     inline void SetCheckPageNum( sal_uInt16 nNew );
diff --git a/sw/source/core/layout/layact.cxx b/sw/source/core/layout/layact.cxx
index 09069261cf71..33b1ae277180 100644
--- a/sw/source/core/layout/layact.cxx
+++ b/sw/source/core/layout/layact.cxx
@@ -319,6 +319,53 @@ bool SwLayAction::RemoveEmptyBrowserPages()
     return bRet;
 }
 
+void SwLayAction::SetAgain(bool bAgain)
+{
+    if (bAgain == m_bAgain)
+        return;
+
+    m_bAgain = bAgain;
+
+    assert(m_aFrameStack.size() == m_aFrameDeleteGuards.size());
+    size_t nCount = m_aFrameStack.size();
+    if (m_bAgain)
+    {
+        // LayAction::FormatLayout is now flagged to exit early and will avoid
+        // dereferencing any SwFrames in the stack of FormatLayouts so allow
+        // their deletion
+        for (size_t i = 0; i < nCount; ++i)
+            m_aFrameDeleteGuards[i].reset();
+    }
+    else
+    {
+        // LayAction::FormatLayout is now continue normally and will
+        // dereference the top SwFrame in the stack of m_aFrameStack as each
+        // FormatLevel returns so disallow their deletion
+        for (size_t i = 0; i < nCount; ++i)
+            m_aFrameDeleteGuards[i] = std::make_unique<SwFrameDeleteGuard>(m_aFrameStack[i]);
+    }
+}
+
+void SwLayAction::PushFormatLayout(SwFrame* pLow)
+{
+    /* Workaround crash seen in crashtesting with fdo53985-1.docx
+
+       Lock pLow against getting deleted when it will be dereferenced
+       after FormatLayout
+
+       If SetAgain is called to make SwLayAction exit early to avoid that
+       dereference, then it clears these guards
+    */
+    m_aFrameStack.push_back(pLow);
+    m_aFrameDeleteGuards.push_back(std::make_unique<SwFrameDeleteGuard>(pLow));
+}
+
+void SwLayAction::PopFormatLayout()
+{
+    m_aFrameDeleteGuards.pop_back();
+    m_aFrameStack.pop_back();
+}
+
 void SwLayAction::Action(OutputDevice* pRenderContext)
 {
     m_bActionInProgress = true;
@@ -1374,7 +1421,11 @@ bool SwLayAction::FormatLayout( OutputDevice *pRenderContext, SwLayoutFrame *pLa
             }
             // Skip the ones already registered for deletion
             else if( !pLow->IsSctFrame() || static_cast<SwSectionFrame*>(pLow)->GetSection() )
+            {
+                PushFormatLayout(pLow);
                 bChanged |= FormatLayout( pRenderContext, static_cast<SwLayoutFrame*>(pLow), bAddRect );
+                PopFormatLayout();
+            }
         }
         else if ( m_pImp->GetShell()->IsPaintLocked() )
             // Shortcut to minimize the cycles. With Lock, the