tdf#101821 sw: fix layout footnote use-after-free

After inserting a header in the bugdoc, during SwFrame::Calc of a
SwTextFrame that is in a footnote, it decides move forward to the next
page.  This deletes the SwFootnoteFrame and SwFootnoteContFrame that
lcl_FormatContentOfLayoutFrame() are iterating over.

For want of a more elegant solution, use a big hammer to prevent the
problem and try to clean up so that no empty SwFootnoteFrame and
SwFootnoteContFrame remain (as that is known to crash in other places,
see commit c9fb347642729017ad0c613fe26310befd021db8)

Invalid read of size 8
   at 0x414E1F96: SwFrame::GetNext() (frame.hxx:485)
   by 0x41AFDD07: lcl_FormatContentOfLayoutFrame(SwLayoutFrame*, SwFrame*) (objectformattertxtfrm.cxx:646)
   by 0x41AFDCC0: lcl_FormatContentOfLayoutFrame(SwLayoutFrame*, SwFrame*) (objectformattertxtfrm.cxx:642)
   by 0x41AFDCC0: lcl_FormatContentOfLayoutFrame(SwLayoutFrame*, SwFrame*) (objectformattertxtfrm.cxx:642)
   by 0x41AFDEA7: SwObjectFormatterTextFrame::FormatAnchorFrameAndItsPrevs(SwTextFrame&) (objectformattertxtfrm.cxx:696)
   by 0x41AAA680: SwFlyAtContentFrame::MakeAll(OutputDevice*) (flycnt.cxx:415)
   by 0x41A7F211: SwFrame::PrepareMake(OutputDevice*) (calcmove.cxx:346)
   by 0x41B75758: SwFrame::Calc(OutputDevice*) const (trvlfrm.cxx:1761)
   by 0x41AA8927: SwFlyFrame::Calc(OutputDevice*) const (fly.cxx:2559)
   by 0x41ADB36B: SwLayAction::FormatLayoutFly(SwFlyFrame*) (layact.cxx:1414)
   by 0x41AF9658: SwObjectFormatter::FormatObj_(SwAnchoredObject&) (objectformatter.cxx:321)
   by 0x41AFCB6E: SwObjectFormatterTextFrame::DoFormatObj(SwAnchoredObject&, bool) (objectformattertxtfrm.cxx:126)
   by 0x41AF9A6A: SwObjectFormatter::FormatObjsAtFrame_(SwTextFrame*) (objectformatter.cxx:443)
   by 0x41AFD275: SwObjectFormatterTextFrame::DoFormatObjs() (objectformattertxtfrm.cxx:328)
   by 0x41AF924A: SwObjectFormatter::FormatObjsAtFrame(SwFrame&, SwPageFrame const&, SwLayAction*) (objectformatter.cxx:191)
   by 0x41ADC213: SwLayAction::FormatContent(SwPageFrame const*) (layact.cxx:1633)
   by 0x41AD88DE: SwLayAction::InternalAction(OutputDevice*) (layact.cxx:760)
   by 0x41AD7080: SwLayAction::Action(OutputDevice*) (layact.cxx:351)
   by 0x41ADE32E: SwLayIdle::SwLayIdle(SwRootFrame*, SwViewShellImp*) (layact.cxx:2133)
   by 0x41FFC97E: SwViewShell::LayoutIdle() (viewsh.cxx:711)
 Address 0x505541a8 is 72 bytes inside a block of size 272 free'd
   at 0x4C2F21A: operator delete(void*) (vg_replace_malloc.c:576)
   by 0x41AD371A: SwFootnoteFrame::~SwFootnoteFrame() (ftnfrm.hxx:52)
   by 0x41B5B74C: SwFrame::DestroyFrame(SwFrame*) (ssfrm.cxx:391)
   by 0x41A97294: SwFlowFrame::CutTree(SwFrame*) (flowfrm.cxx:406)
   by 0x41A979AE: SwFlowFrame::MoveSubTree(SwLayoutFrame*, SwFrame*) (flowfrm.cxx:592)
   by 0x41ACFB69: SwContentFrame::MoveFootnoteCntFwd(bool, SwFootnoteBossFrame*) (ftnfrm.cxx:2756)
   by 0x41A9B78E: SwFlowFrame::MoveFwd(bool, bool, bool) (flowfrm.cxx:1813)
   by 0x41A85864: SwContentFrame::MakeAll(OutputDevice*) (calcmove.cxx:1681)
   by 0x41A7F211: SwFrame::PrepareMake(OutputDevice*) (calcmove.cxx:346)
   by 0x41B75758: SwFrame::Calc(OutputDevice*) const (trvlfrm.cxx:1761)
   by 0x41AFDCFB: lcl_FormatContentOfLayoutFrame(SwLayoutFrame*, SwFrame*) (objectformattertxtfrm.cxx:644)
   by 0x41AFDCC0: lcl_FormatContentOfLayoutFrame(SwLayoutFrame*, SwFrame*) (objectformattertxtfrm.cxx:642)
   by 0x41AFDCC0: lcl_FormatContentOfLayoutFrame(SwLayoutFrame*, SwFrame*) (objectformattertxtfrm.cxx:642)
   by 0x41AFDCC0: lcl_FormatContentOfLayoutFrame(SwLayoutFrame*, SwFrame*) (objectformattertxtfrm.cxx:642)
   by 0x41AFDEA7: SwObjectFormatterTextFrame::FormatAnchorFrameAndItsPrevs(SwTextFrame&) (objectformattertxtfrm.cxx:696)
   by 0x41AAA680: SwFlyAtContentFrame::MakeAll(OutputDevice*) (flycnt.cxx:415)
   by 0x41A7F211: SwFrame::PrepareMake(OutputDevice*) (calcmove.cxx:346)
   by 0x41B75758: SwFrame::Calc(OutputDevice*) const (trvlfrm.cxx:1761)
   by 0x41AA8927: SwFlyFrame::Calc(OutputDevice*) const (fly.cxx:2559)
   by 0x41ADB36B: SwLayAction::FormatLayoutFly(SwFlyFrame*) (layact.cxx:1414)
   by 0x41AF9658: SwObjectFormatter::FormatObj_(SwAnchoredObject&) (objectformatter.cxx:321)
   by 0x41AFCB6E: SwObjectFormatterTextFrame::DoFormatObj(SwAnchoredObject&, bool) (objectformattertxtfrm.cxx:126)
   by 0x41AF9A6A: SwObjectFormatter::FormatObjsAtFrame_(SwTextFrame*) (objectformatter.cxx:443)
   by 0x41AFD275: SwObjectFormatterTextFrame::DoFormatObjs() (objectformattertxtfrm.cxx:328)
   by 0x41AF924A: SwObjectFormatter::FormatObjsAtFrame(SwFrame&, SwPageFrame const&, SwLayAction*) (objectformatter.cxx:191)
   by 0x41ADC213: SwLayAction::FormatContent(SwPageFrame const*) (layact.cxx:1633)
   by 0x41AD88DE: SwLayAction::InternalAction(OutputDevice*) (layact.cxx:760)
   by 0x41AD7080: SwLayAction::Action(OutputDevice*) (layact.cxx:351)
   by 0x41ADE32E: SwLayIdle::SwLayIdle(SwRootFrame*, SwViewShellImp*) (layact.cxx:2133)
   by 0x41FFC97E: SwViewShell::LayoutIdle() (viewsh.cxx:711)

Change-Id: I656cc2303eeccd5eef68ad3b8e81bb0fd47b94fb
(cherry picked from commit 9dcb767c5bdccdf6606240afd6aa2c6bd3dcc9f4)
Reviewed-on: https://gerrit.libreoffice.org/39102
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Miklos Vajna <vmiklos@collabora.co.uk>

2 files changed, 38 insertions, 3 deletions

diff --git a/sw/source/core/layout/flowfrm.cxx b/sw/source/core/layout/flowfrm.cxx
index cf10f8805a96..514e4a43eae5 100644
--- a/sw/source/core/layout/flowfrm.cxx
+++ b/sw/source/core/layout/flowfrm.cxx
@@ -402,8 +402,18 @@ SwLayoutFrame *SwFlowFrame::CutTree( SwFrame *pStart )
         if ( !pLay->Lower() && !pLay->IsColLocked() &&
              !static_cast<SwFootnoteFrame*>(pLay)->IsBackMoveLocked() )
         {
-            pLay->Cut();
-            SwFrame::DestroyFrame(pLay);
+            // tdf#101821 don't delete it while iterating over it
+            if (!pLay->IsDeleteForbidden())
+            {
+                pLay->Cut();
+                SwFrame::DestroyFrame(pLay);
+            }
+            // else: assume there is code on the stack to clean up empty
+            // footnote frames
+            // (don't go into the else branch below, it produces a disconnected
+            // footnote with null upper that can be returned by
+            // SwFootnoteBossFrame::FindFootnote() causing null pointer deref
+            // in SwTextFrame::ConnectFootnote()
         }
         else
         {
diff --git a/sw/source/core/layout/objectformattertxtfrm.cxx b/sw/source/core/layout/objectformattertxtfrm.cxx
index 032f0cf9a304..54cb686637e6 100644
--- a/sw/source/core/layout/objectformattertxtfrm.cxx
+++ b/sw/source/core/layout/objectformattertxtfrm.cxx
@@ -29,6 +29,7 @@
 #include <fmtwrapinfluenceonobjpos.hxx>
 #include <fmtfollowtextflow.hxx>
 #include <layact.hxx>
+#include <ftnfrm.hxx>
 
 using namespace ::com::sun::star;
 
@@ -638,12 +639,36 @@ static void lcl_FormatContentOfLayoutFrame( SwLayoutFrame* pLayFrame,
             break;
         }
         if ( pLowerFrame->IsLayoutFrame() )
+        {
+            SwFrameDeleteGuard aCrudeHack(pLowerFrame); // ??? any issue setting this for non-footnote frames?
             lcl_FormatContentOfLayoutFrame( static_cast<SwLayoutFrame*>(pLowerFrame),
                                         pLastLowerFrame );
+        }
         else
             pLowerFrame->Calc(pLowerFrame->getRootFrame()->GetCurrShell()->GetOut());
 
-        pLowerFrame = pLowerFrame->GetNext();
+        // Calc on a SwTextFrame in a footnote can move it to the next page -
+        // deletion of the SwFootnoteFrame was disabled with SwFrameDeleteGuard
+        // but now we have to clean up empty footnote frames to prevent crashes.
+        // Note: check it at this level, not lower: both container and footnote
+        // can be deleted at the same time!
+        SwFrame *const pNext = pLowerFrame->GetNext();
+        if (pLowerFrame->IsFootnoteContFrame())
+        {
+            for (SwFrame * pFootnote = pLowerFrame->GetLower(); pFootnote; )
+            {
+                assert(pFootnote->IsFootnoteFrame());
+                SwFrame *const pNextNote = pFootnote->GetNext();
+                if (!pFootnote->GetLower() && !pFootnote->IsColLocked() &&
+                    !static_cast<SwFootnoteFrame*>(pFootnote)->IsBackMoveLocked())
+                {
+                    pFootnote->Cut();
+                    SwFrame::DestroyFrame(pFootnote);
+                }
+                pFootnote = pNextNote;
+            }
+        }
+        pLowerFrame = pNext;
     }
 }