summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMichael Stahl <mstahl@redhat.com>2017-06-21 12:26:19 +0200
committerMichael Stahl <mstahl@redhat.com>2017-06-22 09:53:06 +0200
commit9dcb767c5bdccdf6606240afd6aa2c6bd3dcc9f4 (patch)
tree5f8be032c114647dc11399750017280d11371d42
parent4c0b3520b66477334a7971dbed7ffcdcd265e749 (diff)
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
-rw-r--r--sw/source/core/layout/flowfrm.cxx14
-rw-r--r--sw/source/core/layout/objectformattertxtfrm.cxx27
2 files changed, 38 insertions, 3 deletions
diff --git a/sw/source/core/layout/flowfrm.cxx b/sw/source/core/layout/flowfrm.cxx
index bfb1b8be3b8b..0d80955af0bd 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;
}
}