From 39ef8c46df7eb7ce4e184a14ad64a2740a16030b Mon Sep 17 00:00:00 2001 From: Caolán McNamara Date: Fri, 16 Jul 2021 12:45:21 +0100 Subject: crashtesting: UaF on layout of fdo53985-1.docx MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reviewed-on: https://gerrit.libreoffice.org/c/core/+/119060 Tested-by: Jenkins Reviewed-by: Caolán McNamara (cherry picked from commit ceb32f59d96a17c3007ed883fb44bc880673c8e0) Change-Id: Id8ca0d277f485347e21bd8d6d68de2a7de13de48 --- sw/source/core/inc/layact.hxx | 8 ++++++- sw/source/core/layout/layact.cxx | 51 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 1 deletion(-) diff --git a/sw/source/core/inc/layact.hxx b/sw/source/core/inc/layact.hxx index 4e9a01d34830..9ca134c535f1 100644 --- a/sw/source/core/inc/layact.hxx +++ b/sw/source/core/inc/layact.hxx @@ -62,6 +62,9 @@ class SwLayAction SwWait *m_pWait; + std::vector m_aFrameStack; + std::vector> 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. @@ -115,6 +118,9 @@ class SwLayAction bool RemoveEmptyBrowserPages(); + void PushFormatLayout(SwFrame* pLow); + void PopFormatLayout(); + inline void CheckIdleEnd(); public: @@ -146,7 +152,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 b4924390af50..e26c4fd3bafe 100644 --- a/sw/source/core/layout/layact.cxx +++ b/sw/source/core/layout/layact.cxx @@ -313,6 +313,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(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(pLow)); +} + +void SwLayAction::PopFormatLayout() +{ + m_aFrameDeleteGuards.pop_back(); + m_aFrameStack.pop_back(); +} + void SwLayAction::Action(OutputDevice* pRenderContext) { m_bActionInProgress = true; @@ -1360,7 +1407,11 @@ bool SwLayAction::FormatLayout( OutputDevice *pRenderContext, SwLayoutFrame *pLa bTabChanged |= FormatLayoutTab( static_cast(pLow), bAddRect ); // Skip the ones already registered for deletion else if( !pLow->IsSctFrame() || static_cast(pLow)->GetSection() ) + { + PushFormatLayout(pLow); bChanged |= FormatLayout( pRenderContext, static_cast(pLow), bAddRect ); + PopFormatLayout(); + } } else if ( m_pImp->GetShell()->IsPaintLocked() ) // Shortcut to minimize the cycles. With Lock, the -- cgit