commit 471212d464f54054f7419ef1890267d0def852d9
author Michael Stahl <Michael.Stahl@cib.de> Tue Sep 04 18:55:13 2018 +0200
committer Michael Stahl <Michael.Stahl@cib.de> Wed Sep 19 10:18:30 2018 +0200
tree 16b90833fc4f410f2d3a8fc6601cf492f0fcfda7
parent 12a841e1f102ff7f47a7281120fe777bf440d2fd

sw: SwRedlineTable::DeleteAndDestroy() is surprisingly dangerous

At least with the randomised test, it can happen that deleting one
redline will recursively delete other redlines that are located in
the hidden content section of the redline, or at least try to and
crash because those have already been deleted before.

The callers will either delete 1 redline, or delete all of them
via DeleteAndDestroyAll(), so put a safer loop into
DeleteAndDestroyAll() and have DeleteAndDestroy() only delete 1.

Change-Id: I9c4225544a43a4a03f4eb7b6f56e7fe848c8ca54

sw/inc/docary.hxx

diff --git a/sw/inc/docary.hxx b/sw/inc/docary.hxx
index 2087138..940789a 100644
--- a/sw/inc/docary.hxx
+++ b/sw/inc/docary.hxx
@@ -345,7 +345,7 @@ public:

    void Remove( size_type nPos );
    void Remove( const SwRangeRedline* p );
    void DeleteAndDestroy( size_type nPos, size_type nLen = 1 );
    void DeleteAndDestroy(size_type nPos);
    void DeleteAndDestroyAll();

    void dumpAsXml(struct _xmlTextWriter* pWriter) const;
@@ -389,7 +389,7 @@ public:

    void Insert( SwExtraRedline* p );

    void DeleteAndDestroy( sal_uInt16 nPos, sal_uInt16 nLen = 1 );
    void DeleteAndDestroy( sal_uInt16 nPos);
    void DeleteAndDestroyAll();

    void dumpAsXml(struct _xmlTextWriter* pWriter) const;

sw/source/core/doc/docredln.cxx

diff --git a/sw/source/core/doc/docredln.cxx b/sw/source/core/doc/docredln.cxx
index da79c09..808b8dd 100644
--- a/sw/source/core/doc/docredln.cxx
+++ b/sw/source/core/doc/docredln.cxx
@@ -609,26 +609,32 @@ void SwRedlineTable::Remove( size_type nP )

void SwRedlineTable::DeleteAndDestroyAll()
{
    DeleteAndDestroy(0, size());
    if (maVector.empty())
        return;
    SwDoc *const pDoc = maVector.front()->GetDoc();
    while (!maVector.empty())
    {
        auto const pRedline = maVector.back();
        maVector.erase(maVector.back());
        LOKRedlineNotification(RedlineNotification::Remove, pRedline);
        delete pRedline;
    }
    if (pDoc && !pDoc->IsInDtor())
    {
        SwViewShell* pSh(pDoc->getIDocumentLayoutAccess().GetCurrentViewShell() );
        if (pSh)
        {
            pSh->InvalidateWindows(SwRect(0, 0, SAL_MAX_INT32, SAL_MAX_INT32));
        }
    }
}

void SwRedlineTable::DeleteAndDestroy( size_type nP, size_type nL )
void SwRedlineTable::DeleteAndDestroy(size_type const nP)
{
    SwDoc* pDoc = nullptr;
    if( !nP && nL && nL == size() )
        pDoc = maVector.front()->GetDoc();

    for( vector_type::const_iterator it = maVector.begin() + nP; it != maVector.begin() + nP + nL; ++it )
    {
        LOKRedlineNotification(RedlineNotification::Remove, *it);
        delete *it;
    }
    maVector.erase( maVector.begin() + nP, maVector.begin() + nP + nL );

    SwViewShell* pSh;
    if( pDoc && !pDoc->IsInDtor() &&
        nullptr != ( pSh = pDoc->getIDocumentLayoutAccess().GetCurrentViewShell() ) )
        pSh->InvalidateWindows( SwRect( 0, 0, SAL_MAX_INT32, SAL_MAX_INT32 ) );
    auto const pRedline = maVector[nP];
    maVector.erase(maVector.begin() + nP);
    LOKRedlineNotification(RedlineNotification::Remove, pRedline);
    delete pRedline;
}

SwRedlineTable::size_type SwRedlineTable::FindNextOfSeqNo( size_type nSttPos ) const
@@ -1887,7 +1893,7 @@ void SwExtraRedlineTable::Insert( SwExtraRedline* p )
    //p->CallDisplayFunc();
}

void SwExtraRedlineTable::DeleteAndDestroy( sal_uInt16 nPos, sal_uInt16 nLen )
void SwExtraRedlineTable::DeleteAndDestroy(sal_uInt16 const nPos)
{
    /*
    SwDoc* pDoc = 0;
@@ -1895,10 +1901,8 @@ void SwExtraRedlineTable::DeleteAndDestroy( sal_uInt16 nPos, sal_uInt16 nLen )
        pDoc = front()->GetDoc();
    */

    for( std::vector<SwExtraRedline*>::iterator it = m_aExtraRedlines.begin() + nPos; it != m_aExtraRedlines.begin() + nPos + nLen; ++it )
        delete *it;

    m_aExtraRedlines.erase( m_aExtraRedlines.begin() + nPos, m_aExtraRedlines.begin() + nPos + nLen );
    delete m_aExtraRedlines[nPos];
    m_aExtraRedlines.erase(m_aExtraRedlines.begin() + nPos);

    /*
    SwViewShell* pSh;
@@ -1910,7 +1914,12 @@ void SwExtraRedlineTable::DeleteAndDestroy( sal_uInt16 nPos, sal_uInt16 nLen )

void SwExtraRedlineTable::DeleteAndDestroyAll()
{
    DeleteAndDestroy(0, m_aExtraRedlines.size());
    while (!m_aExtraRedlines.empty())
    {
        auto const pRedline = m_aExtraRedlines.back();
        m_aExtraRedlines.pop_back();
        delete pRedline;
    }
}

SwExtraRedline::~SwExtraRedline()