From: Caolán McNamara <caolan.mcnamara@collabora.com>
Date: Sun, 26 Apr 2026 17:48:52 +0000 (+0000)
Subject: [PATCH] drop malformed duplicate-id calc change track actions
X-Git-Tag: archive/raspbian/4%7.4.7-1+rpi1+deb12u14^2~2
X-Git-Url: https://dgit.raspbian.org/?a=commitdiff_plain;h=7849cd956f71eaddaaf0b6933e48e683187df1b7;p=libreoffice.git

[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 6cbb7a0d89f..1a96706bee7 100644
--- a/sc/inc/chgtrack.hxx
+++ b/sc/inc/chgtrack.hxx
@@ -1116,7 +1116,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 55530e494fb..174c42745a6 100644
--- a/sc/source/core/tool/chgtrack.cxx
+++ b/sc/source/core/tool/chgtrack.cxx
@@ -2308,10 +2308,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
@@ -2321,6 +2323,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 9ae06a72072..8551eb11c74 100644
--- a/sc/source/filter/xml/XMLChangeTrackingImportHelper.cxx
+++ b/sc/source/filter/xml/XMLChangeTrackingImportHelper.cxx
@@ -722,8 +722,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)
@@ -767,17 +769,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);