sw/source/core/doc/list.cxx | 12 +
xmloff/source/text/XMLTextListBlockContext.cxx | 172 +++++++++++++------------
2 files changed, 101 insertions(+), 83 deletions(-)
New commits:
commit da798460e370a97597ecc9a06634f400c4b2e0cc
Author: Noel Grandin <noel.gran...@collabora.co.uk>
AuthorDate: Thu Dec 8 14:50:54 2022 +0200
Commit: Noel Grandin <noel.gran...@collabora.co.uk>
CommitDate: Thu Dec 8 18:24:22 2022 +0000
crashtesting ooo84576-1.odt
prevent the OOM by detecting cycles in SwList::SwList and throwing an
exception.
(1) However, that means we need to catch the exception in
XMLTextListBlockContext::XMLTextListBlockContext
and undo some registration, otherwise we will get a use-after-free.
The need to catch it is why I'm using an UNO exception here,
it seemed like a bad idea to throw and then catch and std::foo
exception.
(2) this is still not the end of the story, a further exception
is thrown during SwDoc destruction, for which I don't have a
solution.
Change-Id: I48be3d8acbdc0f9ca948a958f1124b158ba77ac0
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/143820
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.gran...@collabora.co.uk>
diff --git a/sw/source/core/doc/list.cxx b/sw/source/core/doc/list.cxx
index aa4c06f473aa..5095d4e6c9a9 100644
--- a/sw/source/core/doc/list.cxx
+++ b/sw/source/core/doc/list.cxx
@@ -33,8 +33,18 @@ SwList::SwList( OUString sListId,
{
// create empty list trees for the document ranges
const SwNode* pNode = rNodes[SwNodeOffset(0)];
+ std::vector<bool> aVisited(static_cast<sal_Int32>(rNodes.Count()), false);
do
{
+ SwNodeOffset nIndex = pNode->GetIndex();
+ if (aVisited[static_cast<sal_Int32>(nIndex)])
+ {
+ // crashtesting ooo84576-1.odt, which manages to trigger a broken
document structure
+ // in our code. This is just a workaround to prevent an infinite
loop leading to OOM.
+ SAL_WARN("sw.core", "corrupt document structure, bailing out of
infinite loop");
+ throw css::uno::RuntimeException("corrupt document structure,
bailing out of infinite loop");
+ }
+ aVisited[static_cast<sal_Int32>(nIndex)] = true;
SwPaM aPam( *pNode, *pNode->EndOfSectionNode() );
maListTrees.emplace_back(
@@ -46,7 +56,7 @@ SwList::SwList( OUString sListId,
pNode = pNode->EndOfSectionNode();
if (pNode != &rNodes.GetEndOfContent())
{
- SwNodeOffset nIndex = pNode->GetIndex();
+ nIndex = pNode->GetIndex();
nIndex++;
pNode = rNodes[nIndex];
}
diff --git a/xmloff/source/text/XMLTextListBlockContext.cxx
b/xmloff/source/text/XMLTextListBlockContext.cxx
index 7c688f4c5e6c..c22eb2e221f2 100644
--- a/xmloff/source/text/XMLTextListBlockContext.cxx
+++ b/xmloff/source/text/XMLTextListBlockContext.cxx
@@ -110,113 +110,121 @@ XMLTextListBlockContext::XMLTextListBlockContext(
// Remember this list block.
mrTxtImport.GetTextListHelper().PushListContext( this );
+ try
+ {
+ mxNumRules = XMLTextListsHelper::MakeNumRule(GetImport(), mxNumRules,
+ sParentListStyleName, msListStyleName,
+ mnLevel, &mbRestartNumbering, &mbSetDefaults );
+ if( !mxNumRules.is() )
+ return;
- mxNumRules = XMLTextListsHelper::MakeNumRule(GetImport(), mxNumRules,
- sParentListStyleName, msListStyleName,
- mnLevel, &mbRestartNumbering, &mbSetDefaults );
- if( !mxNumRules.is() )
- return;
-
- if ( mnLevel != 0 ) // root <list> element
- return;
+ if ( mnLevel != 0 ) // root <list> element
+ return;
- XMLTextListsHelper& rTextListsHelper( mrTxtImport.GetTextListHelper() );
- // Inconsistent behavior regarding lists (#i92811#)
- OUString sListStyleDefaultListId;
- {
- uno::Reference< beans::XPropertySet > xNumRuleProps( mxNumRules,
UNO_QUERY );
- if ( xNumRuleProps.is() )
+ XMLTextListsHelper& rTextListsHelper( mrTxtImport.GetTextListHelper()
);
+ // Inconsistent behavior regarding lists (#i92811#)
+ OUString sListStyleDefaultListId;
{
- uno::Reference< beans::XPropertySetInfo > xNumRulePropSetInfo(
- xNumRuleProps->getPropertySetInfo());
- if (xNumRulePropSetInfo.is() &&
- xNumRulePropSetInfo->hasPropertyByName(
- s_PropNameDefaultListId))
+ uno::Reference< beans::XPropertySet > xNumRuleProps( mxNumRules,
UNO_QUERY );
+ if ( xNumRuleProps.is() )
{
- xNumRuleProps->getPropertyValue(s_PropNameDefaultListId)
- >>= sListStyleDefaultListId;
- SAL_WARN_IF( sListStyleDefaultListId.isEmpty(), "xmloff",
- "no default list id found at numbering rules
instance. Serious defect." );
+ uno::Reference< beans::XPropertySetInfo > xNumRulePropSetInfo(
+
xNumRuleProps->getPropertySetInfo());
+ if (xNumRulePropSetInfo.is() &&
+ xNumRulePropSetInfo->hasPropertyByName(
+ s_PropNameDefaultListId))
+ {
+ xNumRuleProps->getPropertyValue(s_PropNameDefaultListId)
+ >>= sListStyleDefaultListId;
+ SAL_WARN_IF( sListStyleDefaultListId.isEmpty(), "xmloff",
+ "no default list id found at numbering rules
instance. Serious defect." );
+ }
}
}
- }
- if ( msListId.isEmpty() ) // no text:id property found
- {
- sal_Int32 nUPD( 0 );
- sal_Int32 nBuild( 0 );
- const bool bBuildIdFound = GetImport().getBuildIds( nUPD, nBuild );
- if ( rImport.IsTextDocInOOoFileFormat() ||
- ( bBuildIdFound && nUPD == 680 ) )
+ if ( msListId.isEmpty() ) // no text:id property found
{
- /* handling former documents written by OpenOffice.org:
- use default list id of numbering rules instance, if existing
- (#i92811#)
- */
- if ( !sListStyleDefaultListId.isEmpty() )
+ sal_Int32 nUPD( 0 );
+ sal_Int32 nBuild( 0 );
+ const bool bBuildIdFound = GetImport().getBuildIds( nUPD, nBuild );
+ if ( rImport.IsTextDocInOOoFileFormat() ||
+ ( bBuildIdFound && nUPD == 680 ) )
{
- msListId = sListStyleDefaultListId;
- if ( !bIsContinueNumberingAttributePresent &&
- !mbRestartNumbering &&
- rTextListsHelper.IsListProcessed( msListId ) )
+ /* handling former documents written by OpenOffice.org:
+ use default list id of numbering rules instance, if existing
+ (#i92811#)
+ */
+ if ( !sListStyleDefaultListId.isEmpty() )
{
- mbRestartNumbering = true;
+ msListId = sListStyleDefaultListId;
+ if ( !bIsContinueNumberingAttributePresent &&
+ !mbRestartNumbering &&
+ rTextListsHelper.IsListProcessed( msListId ) )
+ {
+ mbRestartNumbering = true;
+ }
}
}
+ if ( msListId.isEmpty() )
+ {
+ // generate a new list id for the list
+ msListId = rTextListsHelper.GenerateNewListId();
+ }
}
- if ( msListId.isEmpty() )
- {
- // generate a new list id for the list
- msListId = rTextListsHelper.GenerateNewListId();
- }
- }
- if ( bIsContinueNumberingAttributePresent && !mbRestartNumbering &&
- msContinueListId.isEmpty() )
- {
- const OUString& Last( rTextListsHelper.GetLastProcessedListId() );
- if ( rTextListsHelper.GetListStyleOfLastProcessedList() ==
msListStyleName
- && Last != msListId )
+ if ( bIsContinueNumberingAttributePresent && !mbRestartNumbering &&
+ msContinueListId.isEmpty() )
{
- msContinueListId = Last;
+ const OUString& Last( rTextListsHelper.GetLastProcessedListId() );
+ if ( rTextListsHelper.GetListStyleOfLastProcessedList() ==
msListStyleName
+ && Last != msListId )
+ {
+ msContinueListId = Last;
+ }
}
- }
- bool bContinueNumbering = bIsContinueNumberingAttributePresent &&
!mbRestartNumbering;
- if (msContinueListId.isEmpty() && bContinueNumbering &&
GetImport().IsMSO())
- {
- // No "continue list" id, but continue numbering was requested.
Connect to the last list of
- // the same list style in the Word case, even if there was a different
list in the meantime.
- msContinueListId =
rTextListsHelper.GetLastIdOfStyleName(msListStyleName);
- }
-
- if ( !msContinueListId.isEmpty() )
- {
- if ( !rTextListsHelper.IsListProcessed( msContinueListId ) )
+ bool bContinueNumbering = bIsContinueNumberingAttributePresent &&
!mbRestartNumbering;
+ if (msContinueListId.isEmpty() && bContinueNumbering &&
GetImport().IsMSO())
{
- msContinueListId.clear();
+ // No "continue list" id, but continue numbering was requested.
Connect to the last list of
+ // the same list style in the Word case, even if there was a
different list in the meantime.
+ msContinueListId =
rTextListsHelper.GetLastIdOfStyleName(msListStyleName);
}
- else
+
+ if ( !msContinueListId.isEmpty() )
{
- // search continue list chain for master list and
- // continue the master list.
- OUString sTmpStr =
- rTextListsHelper.GetContinueListIdOfProcessedList(
msContinueListId );
- while ( !sTmpStr.isEmpty() )
+ if ( !rTextListsHelper.IsListProcessed( msContinueListId ) )
{
- msContinueListId = sTmpStr;
-
- sTmpStr =
+ msContinueListId.clear();
+ }
+ else
+ {
+ // search continue list chain for master list and
+ // continue the master list.
+ OUString sTmpStr =
rTextListsHelper.GetContinueListIdOfProcessedList(
msContinueListId );
+ while ( !sTmpStr.isEmpty() )
+ {
+ msContinueListId = sTmpStr;
+
+ sTmpStr =
+ rTextListsHelper.GetContinueListIdOfProcessedList(
msContinueListId );
+ }
}
}
- }
- if ( !rTextListsHelper.IsListProcessed( msListId ) )
+ if ( !rTextListsHelper.IsListProcessed( msListId ) )
+ {
+ // Inconsistent behavior regarding lists (#i92811#)
+ rTextListsHelper.KeepListAsProcessed(
+ msListId, msListStyleName, msContinueListId,
+ sListStyleDefaultListId );
+ }
+ }
+ catch (uno::Exception&)
{
- // Inconsistent behavior regarding lists (#i92811#)
- rTextListsHelper.KeepListAsProcessed(
- msListId, msListStyleName, msContinueListId,
- sListStyleDefaultListId );
+ // pop ourselves if anything goes wrong to avoid use-after-free
+ rTxtImp.GetTextListHelper().PopListContext();
+ throw;
}
}
