[PATCH] drop malformed duplicate-id calc change track actions

Signed-off-by: Caolán McNamara <caolan.mcnamara@collabora.com>
Change-Id: I0d532eb84d122e39ce71e94b4760b94cd636eeea
Reviewed-on: https://gerrit.collaboraoffice.com/c/online/+/1691
Tested-by: Jenkins CPCI <releng@collaboraoffice.com>
Reviewed-by: Miklos Vajna <vmiklos@collabora.com>
(cherry picked from commit b5b56a2239369e45d7fac2ea965558a1f337b602)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/205091
Reviewed-by: Xisco Fauli <xiscofauli@libreoffice.org>
Tested-by: Jenkins
(cherry picked from commit a20ae7b5f80a45e49a47ce22f5e92749cd9816a0)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/205101

Gbp-Pq: Name CVE-2026-8358.diff

diff --git a/sc/inc/chgtrack.hxx b/sc/inc/chgtrack.hxx
index 9e3aed17aa554e2ae57f25a28b5d661e06ca1578..7c1889e18bab2b1b8b3d3ec525bf481a9eb75184 100644 (file)
--- a/sc/inc/chgtrack.hxx
+++ b/sc/inc/chgtrack.hxx
@@ -1115,7 +1115,8 @@ public:
 
     sal_uLong           AddLoadedGenerated( const ScCellValue& rNewCell,
                             const ScBigRange& aBigRange, const OUString& sNewValue ); // only to use in the XML import
-    void                AppendLoaded( std::unique_ptr<ScChangeAction> pAppend ); // this is only for the XML import public, it should be protected
+    // returns false if the action number is already in use, in which case the new duplicate is dropped
+    bool                AppendLoaded( std::unique_ptr<ScChangeAction> pAppend ); // this is only for the XML import public, it should be protected
     void                SetActionMax(sal_uLong nTempActionMax)
                             { nActionMax = nTempActionMax; } // only to use in the XML import
 

diff --git a/sc/source/core/tool/chgtrack.cxx b/sc/source/core/tool/chgtrack.cxx
index f5ccfd579afb3a566f63742e030eafaa7b8629c4..ee3b850e0bd4ecb4a2dee2074a59da0967864d86 100644 (file)
--- a/sc/source/core/tool/chgtrack.cxx
+++ b/sc/source/core/tool/chgtrack.cxx
@@ -2315,10 +2315,12 @@ void ScChangeTrack::MasterLinks( ScChangeAction* pAppend )
     }
 }
 
-void ScChangeTrack::AppendLoaded( std::unique_ptr<ScChangeAction> pActionParam )
+bool ScChangeTrack::AppendLoaded( std::unique_ptr<ScChangeAction> pActionParam )
 {
+    auto [it, inserted] = aMap.insert(std::make_pair(pActionParam->GetActionNumber(), pActionParam.get()));
+    if (!inserted)
+        return false;
     ScChangeAction* pAppend = pActionParam.release();
-    aMap.insert( ::std::make_pair( pAppend->GetActionNumber(), pAppend ) );
     if ( !pLast )
         pFirst = pLast = pAppend;
     else
@@ -2328,6 +2330,7 @@ void ScChangeTrack::AppendLoaded( std::unique_ptr<ScChangeAction> pActionParam )
         pLast = pAppend;
     }
     MasterLinks( pAppend );
+    return true;
 }
 
 void ScChangeTrack::Append( ScChangeAction* pAppend, sal_uLong nAction )

diff --git a/sc/source/filter/xml/XMLChangeTrackingImportHelper.cxx b/sc/source/filter/xml/XMLChangeTrackingImportHelper.cxx
index 1bc9c21b0e6fd0d21fa253de07fae2c792759a10..b4b747f4ef367d255971d673f36ed530412c8c8b 100644 (file)
--- a/sc/source/filter/xml/XMLChangeTrackingImportHelper.cxx
+++ b/sc/source/filter/xml/XMLChangeTrackingImportHelper.cxx
@@ -719,8 +719,10 @@ void ScXMLChangeTrackingImportHelper::CreateChangeTrack(ScDocument* pDoc)
     // old files didn't store nanoseconds, disable until encountered
     pTrack->SetTimeNanoSeconds( false );
 
-    for (const auto & rAction : aActions)
+    auto aItr = aActions.begin();
+    while (aItr != aActions.end())
     {
+        const auto& rAction = *aItr;
         std::unique_ptr<ScChangeAction> pAction;
 
         switch (rAction->nActionType)
@@ -764,17 +766,20 @@ void ScXMLChangeTrackingImportHelper::CreateChangeTrack(ScDocument* pDoc)
             }
         }
 
-        if (pAction)
-            pTrack->AppendLoaded(std::move(pAction));
+        // Malformed documents can repeat the same XML id across actions. If
+        // this happens drop entries whose action number is already in the track.
+        if (pAction && pTrack->AppendLoaded(std::move(pAction)))
+            ++aItr;
         else
         {
-            OSL_FAIL("no action");
+            SAL_WARN("sc.filter", "Dropping malformed change track entry");
+            aItr = aActions.erase(aItr);
         }
     }
     if (pTrack->GetLast())
         pTrack->SetActionMax(pTrack->GetLast()->GetActionNumber());
 
-    auto aItr = aActions.begin();
+    aItr = aActions.begin();
     while (aItr != aActions.end())
     {
         SetDependencies(aItr->get(), *pDoc);