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

[Caolán McNamara (via logerrit)]

 sw/source/core/inc/frame.hxx    |   15 ++-------------
 sw/source/core/layout/ssfrm.cxx |   30 +++++++++++++++++++++++++++++-
 2 files changed, 31 insertions(+), 14 deletions(-)

New commits:
commit ee8c90c469c2caeca24a9621847ec796d011f3bf
Author:     Caolán McNamara <caolanm at redhat.com>
AuthorDate: Wed Apr 24 20:36:50 2019 +0100
Commit:     Caolán McNamara <caolanm at redhat.com>
CommitDate: Thu Apr 25 09:31:30 2019 +0200

    ubsan: use after free
    
    Change-Id: I1a3396d434f6cc5bb615bcb2fe06a819bc11c013
    Reviewed-on: https://gerrit.libreoffice.org/71256
    Tested-by: Jenkins
    Reviewed-by: Caolán McNamara <caolanm at redhat.com>
    Tested-by: Caolán McNamara <caolanm at redhat.com>

diff --git a/sw/source/core/inc/frame.hxx b/sw/source/core/inc/frame.hxx
index 6e0d653690c4..ab1047d0348f 100644
--- a/sw/source/core/inc/frame.hxx
+++ b/sw/source/core/inc/frame.hxx
@@ -1237,19 +1237,8 @@ public:
     //Flag pFrame for SwFrameDeleteGuard lifetime that we shouldn't delete
     //it in e.g. SwSectionFrame::MergeNext etc because we will need it
     //again after the SwFrameDeleteGuard dtor
-    explicit SwFrameDeleteGuard(SwFrame* pFrame)
-        : m_pForbidFrame((pFrame && !pFrame->IsDeleteForbidden()) ?
-            pFrame : nullptr)
-    {
-        if (m_pForbidFrame)
-            m_pForbidFrame->ForbidDelete();
-    }
-
-    ~SwFrameDeleteGuard()
-    {
-        if (m_pForbidFrame)
-            m_pForbidFrame->AllowDelete();
-    }
+    explicit SwFrameDeleteGuard(SwFrame* pFrame);
+    ~SwFrameDeleteGuard();
 };
 
 typedef long (SwFrame:: *SwFrameGet)() const;
diff --git a/sw/source/core/layout/ssfrm.cxx b/sw/source/core/layout/ssfrm.cxx
index 741b942c6850..9e496a1bd2ec 100644
--- a/sw/source/core/layout/ssfrm.cxx
+++ b/sw/source/core/layout/ssfrm.cxx
@@ -468,6 +468,33 @@ void SwTextFrame::RegisterToNode(SwTextNode & rNode, bool const isForceNodeAsFir
     }
 }
 
+//Flag pFrame for SwFrameDeleteGuard lifetime that we shouldn't delete
+//it in e.g. SwSectionFrame::MergeNext etc because we will need it
+//again after the SwFrameDeleteGuard dtor
+SwFrameDeleteGuard::SwFrameDeleteGuard(SwFrame* pFrame)
+    : m_pForbidFrame((pFrame && !pFrame->IsDeleteForbidden()) ? pFrame : nullptr)
+{
+    if (m_pForbidFrame)
+    {
+        m_pForbidFrame->ForbidDelete();
+    }
+}
+
+SwFrameDeleteGuard::~SwFrameDeleteGuard()
+{
+    if (m_pForbidFrame)
+    {
+        const bool bLogicErrorThrown = !m_pForbidFrame->IsDeleteForbidden();
+        if (bLogicErrorThrown)
+        {
+            // see testForcepoint80
+            SwFrame::DestroyFrame(m_pForbidFrame);
+            return;
+        }
+        m_pForbidFrame->AllowDelete();
+    }
+}
+
 void SwLayoutFrame::DestroyImpl()
 {
     while (!m_VertPosOrientFramesFor.empty())
@@ -527,7 +554,8 @@ void SwLayoutFrame::DestroyImpl()
                 pFrame->AllowDelete();
                 bFatalError = true;
             }
-            SwFrame::DestroyFrame(pFrame);
+            else
+                SwFrame::DestroyFrame(pFrame);
             pFrame = m_pLower;
         }