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

[Michael Stahl (via logerrit)]

 sw/source/core/text/frmform.cxx |   10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

New commits:
commit 95a716f12a0dacdd5d80a6f3d98eadcc2f079fcf
Author:     Michael Stahl <michael.stahl at allotropia.de>
AuthorDate: Fri May 21 12:20:28 2021 +0200
Commit:     Michael Stahl <michael.stahl at allotropia.de>
CommitDate: Fri May 21 14:32:44 2021 +0200

    sw: layout: fix footnote UAF on tdf132248-3.odt (also, tdf134127-1.odt)
    
    Text frame 12 gets a follow 137 which gets a follow 138, and there's
    a footnote at index 95.
    
    When 138 is created, its mnOffest is 63 in SwTextFrame::SplitFrame()
    so 138 gets its mbFootnote set and the footnote frame mpReference points
    to 138 but then SwTextFrame::FormatAdjust() moves one line back and sets
    the offset of 138 to 96.
    
    Then SwTextFrame::CalcFollow() clears 138's mbFootnote due to lack of
    footnotes starting from 96.
    
    Later frame 138 is joined, but because its mbFootnote is false, the
    footnote's mpReference continues to point to it, which leads to UAF in
    SwFootnoteFrame::GetRef().
    
    What's missing is that at the time when the offset of 138 is adjusted,
    RemoveFootnote() must be called, as is already the case in another
    branch of SwTextFrame::FormatAdjust(), since CVS initial import.
    
    Not sure why this started to crash in crashtesting relatively recently.
    
    Change-Id: Ic46019d34ff90d24d0f23effe8a6d54d6f197a71
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/115910
    Tested-by: Jenkins
    Reviewed-by: Michael Stahl <michael.stahl at allotropia.de>

diff --git a/sw/source/core/text/frmform.cxx b/sw/source/core/text/frmform.cxx
index d461c938739c..12a4492bbec7 100644
--- a/sw/source/core/text/frmform.cxx
+++ b/sw/source/core/text/frmform.cxx
@@ -987,7 +987,7 @@ bool SwTextFrame::CalcPreps()
     return bRet;
 }
 
-// We rewire the footnotes and the character bound objects
+// Move the as-character objects - footnotes must be moved by RemoveFootnote!
 void SwTextFrame::ChangeOffset( SwTextFrame* pFrame, TextFrameIndex nNew )
 {
     if( pFrame->GetOffset() < nNew )
@@ -1082,6 +1082,7 @@ void SwTextFrame::FormatAdjust( SwTextFormatter &rLine,
         // need to create a Follow.
         // We also need to do this if the whole mass of text remains in the Master,
         // because a hard line break could necessitate another line (without text mass)!
+        TextFrameIndex const nOld(nEnd);
         nEnd = rLine.GetEnd();
         if( GetFollow() )
         {
@@ -1107,6 +1108,13 @@ void SwTextFrame::FormatAdjust( SwTextFormatter &rLine,
                 // for the paragraph mark.
                 nNew |= 1;
             }
+            // move footnotes if the follow is kept - if RemoveFootnote() is
+            // called in next format iteration, it will be with the *new*
+            // offset so no effect!
+            if (nNew && nOld < nEnd)
+            {
+                RemoveFootnote(nOld, nEnd - nOld);
+            }
             ChangeOffset( GetFollow(), nEnd );
             GetFollow()->ManipOfst( nEnd );
         }