From 0e0bad1a6a3affa2b3fd82cc3834ae03ea7bc1d5 Mon Sep 17 00:00:00 2001 From: Miklos Vajna Date: Fri, 15 Nov 2019 12:14:09 +0100 Subject: [PATCH] tdf#128736 sw ContinuousEndnotes: fix use-after-free on text frame join Regression from commit 61cf196631a2a846e0d3b8b83c0805cf4d1d14b2 (sw ContinuousEndnotes: fix moving them to the previous page, 2019-10-25), the problem was that the SwFootnoteFrame::mpReference was not updated on frame split. This meant that by the time the frame was joined, SwTextFrame::JoinFrame() thought that the follow has no footnotes, so the footnote reference was not updated, resulting in a dangling pointer. Fix the problem by going back to using bEnd for endnotes (both the Word compat and the normal case), this means that SwTextFrame::ConnectFootnote() will invoke SwFootnoteBossFrame::ChangeFootnoteRef(), fixing the dangling pointer. Then fix the original problem differently: similar to the in-section endnotes, just remove + append them, this makes sure that the endnotes (in the Word compat case) move to a previous page on page delete. Change-Id: Ia1b8e54b4a0b0f385c703f8e7016011c3ac57a03 Reviewed-on: https://gerrit.libreoffice.org/82778 Reviewed-by: Miklos Vajna Tested-by: Jenkins --- sw/CppunitTest_sw_core_text.mk | 68 +++++++++++ sw/Module_sw.mk | 1 + sw/qa/core/text/data/footnote-connect.fodt | 132 +++++++++++++++++++++ sw/qa/core/text/text.cxx | 51 ++++++++ sw/source/core/text/txtftn.cxx | 10 +- 5 files changed, 259 insertions(+), 3 deletions(-) create mode 100644 sw/CppunitTest_sw_core_text.mk create mode 100644 sw/qa/core/text/data/footnote-connect.fodt create mode 100644 sw/qa/core/text/text.cxx diff --git a/sw/CppunitTest_sw_core_text.mk b/sw/CppunitTest_sw_core_text.mk new file mode 100644 index 000000000000..9fe4fadbf357 --- /dev/null +++ b/sw/CppunitTest_sw_core_text.mk @@ -0,0 +1,68 @@ +# -*- Mode: makefile-gmake; tab-width: 4; indent-tabs-mode: t -*- +#************************************************************************* +# +# This file is part of the LibreOffice project. +# +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, You can obtain one at http://mozilla.org/MPL/2.0/. +# +#************************************************************************* + +$(eval $(call gb_CppunitTest_CppunitTest,sw_core_text)) + +$(eval $(call gb_CppunitTest_use_common_precompiled_header,sw_core_text)) + +$(eval $(call gb_CppunitTest_add_exception_objects,sw_core_text, \ + sw/qa/core/text/text \ +)) + +$(eval $(call gb_CppunitTest_use_libraries,sw_core_text, \ + comphelper \ + cppu \ + cppuhelper \ + sal \ + sfx \ + sw \ + test \ + unotest \ + utl \ +)) + +$(eval $(call gb_CppunitTest_use_externals,sw_core_text,\ + boost_headers \ + libxml2 \ +)) + +$(eval $(call gb_CppunitTest_set_include,sw_core_text,\ + -I$(SRCDIR)/sw/inc \ + -I$(SRCDIR)/sw/source/core/inc \ + -I$(SRCDIR)/sw/source/uibase/inc \ + -I$(SRCDIR)/sw/qa/inc \ + $$(INCLUDE) \ +)) + +$(eval $(call gb_CppunitTest_use_api,sw_core_text,\ + udkapi \ + offapi \ + oovbaapi \ +)) + +$(eval $(call gb_CppunitTest_use_ure,sw_core_text)) +$(eval $(call gb_CppunitTest_use_vcl,sw_core_text)) + +$(eval $(call gb_CppunitTest_use_rdb,sw_core_text,services)) + +$(eval $(call gb_CppunitTest_use_custom_headers,sw_core_text,\ + officecfg/registry \ +)) + +$(eval $(call gb_CppunitTest_use_configuration,sw_core_text)) + +$(eval $(call gb_CppunitTest_use_uiconfigs,sw_core_text, \ + modules/swriter \ +)) + +$(eval $(call gb_CppunitTest_use_more_fonts,sw_core_text)) + +# vim: set noet sw=4 ts=4: diff --git a/sw/Module_sw.mk b/sw/Module_sw.mk index b16a4aea6e16..2dbd5f161f22 100644 --- a/sw/Module_sw.mk +++ b/sw/Module_sw.mk @@ -104,6 +104,7 @@ $(eval $(call gb_Module_add_slowcheck_targets,sw,\ CppunitTest_sw_accessible_relation_set \ CppunitTest_sw_apitests \ CppunitTest_sw_unowriter \ + CppunitTest_sw_core_text \ )) ifneq ($(DISABLE_GUI),TRUE) diff --git a/sw/qa/core/text/data/footnote-connect.fodt b/sw/qa/core/text/data/footnote-connect.fodt new file mode 100644 index 000000000000..dd26db98c9e0 --- /dev/null +++ b/sw/qa/core/text/data/footnote-connect.fodt @@ -0,0 +1,132 @@ + + + + + false + true + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + A1 + + + + + + P01 + P02 + P03 + P04 + P05 + P06 + P07 + P08 + P09 + P10 + P11 + P12 + P13 + P14 + P15 + P16 + P17 + P18 + P19 + P20 + P21 + P22 + P23 + P24 + P25 + P26 + P27 + P28 + P29 + P30 + P31 + P32 + P33 + P34 + P35 + P36 + P37 + P38 + + + + P2P01 + P2P02 + P2P03 + P2P04 + P2P05 + P2P06 + P2P07 + P2P08 + P2P09 + P2P10 + P2P11 + P2P121 First linesecond line + + + + diff --git a/sw/qa/core/text/text.cxx b/sw/qa/core/text/text.cxx new file mode 100644 index 000000000000..8074cbc9f45e --- /dev/null +++ b/sw/qa/core/text/text.cxx @@ -0,0 +1,51 @@ +/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */ +/* + * This file is part of the LibreOffice project. + * + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. + */ + +#include +#include + +static char const DATA_DIRECTORY[] = "/sw/qa/core/text/data/"; + +/// Covers sw/source/core/text/ fixes. +class SwCoreTextTest : public SwModelTestBase +{ +public: + SwDoc* createDoc(const char* pName = nullptr); +}; + +SwDoc* SwCoreTextTest::createDoc(const char* pName) +{ + load(DATA_DIRECTORY, pName); + + SwXTextDocument* pTextDoc = dynamic_cast(mxComponent.get()); + CPPUNIT_ASSERT(pTextDoc); + return pTextDoc->GetDocShell()->GetDoc(); +} + +CPPUNIT_TEST_FIXTURE(SwCoreTextTest, testFootnoteConnect) +{ + SwDoc* pDoc = createDoc("footnote-connect.fodt"); + SwWrtShell* pWrtShell = pDoc->GetDocShell()->GetWrtShell(); + // Jump to the start of the next page. + pWrtShell->SttNxtPg(); + // Remove the page break. + pWrtShell->DelLeft(); + // Split the multi-line text frame, containing an endnote. + pWrtShell->DelLeft(); + // Join the split text frame. + pWrtShell->DelLeft(); + // Turn the 3 page document into a 2 page one, so the endnote frame is moved. + // Without the accompanying fix in place, this test would have crashed due to a use-after-free + // in SwFootnoteFrame::GetRef(). + pWrtShell->DelLeft(); +} + +CPPUNIT_PLUGIN_IMPLEMENT(); + +/* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/sw/source/core/text/txtftn.cxx b/sw/source/core/text/txtftn.cxx index 9bc253768349..4ca6938e82f3 100644 --- a/sw/source/core/text/txtftn.cxx +++ b/sw/source/core/text/txtftn.cxx @@ -585,7 +585,7 @@ void SwTextFrame::ConnectFootnote( SwTextFootnote *pFootnote, const SwTwips nDea // See if pFootnote is an endnote on a separate endnote page. const IDocumentSettingAccess& rSettings = GetDoc().getIDocumentSettingAccess(); bool bContinuousEndnotes = rSettings.get(DocumentSettingId::CONTINUOUS_ENDNOTES); - const bool bEnd = pFootnote->GetFootnote().IsEndNote() && !bContinuousEndnotes; + const bool bEnd = pFootnote->GetFootnote().IsEndNote(); // We want to store this value, because it is needed as a fallback // in GetFootnoteLine(), if there is no paragraph information available @@ -615,11 +615,15 @@ void SwTextFrame::ConnectFootnote( SwTextFootnote *pFootnote, const SwTwips nDea if( bDocEnd ) { - if( pSect && pSrcFrame ) + if ((pSect || bContinuousEndnotes) && pSrcFrame) { SwFootnoteFrame *pFootnoteFrame = SwFootnoteBossFrame::FindFootnote( pSrcFrame, pFootnote ); - if( pFootnoteFrame && pFootnoteFrame->IsInSct() ) + if (pFootnoteFrame && (pFootnoteFrame->IsInSct() || bContinuousEndnotes)) { + // We either have a foot/endnote that goes to the end of the section or are in Word + // compatibility mode where endnotes go to the end of the document. Handle both + // cases by removing the footnote here, then later appending them to the correct + // last page of the document or section. pBoss->RemoveFootnote( pSrcFrame, pFootnote ); pSrcFrame = nullptr; }