From 65dbd776fb10ddffafa2e507248603afeca52399 Mon Sep 17 00:00:00 2001
From: wmayer <wmayer@users.sourceforge.net>
Date: Tue, 30 Apr 2024 15:57:01 +0200
Subject: [PATCH] Core: Fix crash when loading a file with v0.21 that was
 created with v0.22

In v0.22 a version number -1 is added to the material files to distinguish between old and new project file.
But v0.21 doesn't know about this version number and interprets it as number of elements instead.
Because this value is assigned to an unsigned type the value becomes 2**32 - 1. Now trying to create a container of this size requires > 280 GB
of RAM. On most systems FreeCAD new handler will jump in and raises a memory exception to stop the allocation. But an other systems with
plenty of RAM it's tried to allocate the memory and then may crash at some point.

This PR fixes this regression. It puts the version number to the MaterialList XML element as an optional attribute.
With this change FreeCAD v0.22 is still able to load projects that have been created prior to this change.
Additionally FreeCAD v0.21 can again load project files without crashing.

For more details see: https://forum.freecad.org/viewtopic.php?t=87268
---
 src/App/PropertyStandard.cpp | 38 ++++++++++++++++++++++--------------
 src/App/PropertyStandard.h   | 10 +++++++++-
 2 files changed, 32 insertions(+), 16 deletions(-)

diff --git a/src/App/PropertyStandard.cpp b/src/App/PropertyStandard.cpp
index 3117e5e739..c2d77a24e6 100644
--- a/src/App/PropertyStandard.cpp
+++ b/src/App/PropertyStandard.cpp
@@ -3072,7 +3072,8 @@ void PropertyMaterialList::Save(Base::Writer& writer) const
 {
     if (!writer.isForceXML()) {
         writer.Stream() << writer.ind() << "<MaterialList file=\""
-                        << (getSize() ? writer.addFile(getName(), this) : "") << "\"/>"
+                        << (getSize() ? writer.addFile(getName(), this) : "") << "\""
+                        << " version=\"2\"/>"
                         << std::endl;
     }
 }
@@ -3082,6 +3083,9 @@ void PropertyMaterialList::Restore(Base::XMLReader& reader)
     reader.readElement("MaterialList");
     if (reader.hasAttribute("file")) {
         std::string file(reader.getAttribute("file"));
+        if (reader.hasAttribute("version")) {
+            formatVersion = static_cast<Format>(reader.getAttributeAsInteger("version"));
+        }
 
         if (!file.empty()) {
             // initiate a file read
@@ -3093,10 +3097,6 @@ void PropertyMaterialList::Restore(Base::XMLReader& reader)
 void PropertyMaterialList::SaveDocFile(Base::Writer& writer) const
 {
     Base::OutputStream str(writer.Stream());
-    // Write the version. Versions should be negative. A non-negative value is a count
-    // and should be processed as a V0
-    int32_t version = -1;
-    str << version;
     uint32_t uCt = (uint32_t)getSize();
     str << uCt;
     for (const auto& it : _lValueList) {
@@ -3113,14 +3113,22 @@ void PropertyMaterialList::SaveDocFile(Base::Writer& writer) const
 void PropertyMaterialList::RestoreDocFile(Base::Reader& reader)
 {
     Base::InputStream str(reader);
-    int32_t version;
-    str >> version;
-    if (version < 0) {
-        RestoreDocFileV1(reader);
+    if (formatVersion == Version_2) {
+        // V2 is same as V0
+        uint32_t count = 0;
+        str >> count;
+        RestoreDocFileV0(count, reader);
     }
     else {
-        uint32_t uCt = static_cast<uint32_t>(version);
-        RestoreDocFileV0(uCt, reader);
+        int32_t version;
+        str >> version;
+        if (version < 0) {
+            RestoreDocFileV1(reader);
+        }
+        else {
+            uint32_t uCt = static_cast<uint32_t>(version);
+            RestoreDocFileV0(uCt, reader);
+        }
     }
 }
 
@@ -3128,8 +3136,8 @@ void PropertyMaterialList::RestoreDocFileV0(uint32_t count, Base::Reader& reader
 {
     Base::InputStream str(reader);
     std::vector<Material> values(count);
-    uint32_t value;  // must be 32 bit long
-    float valueF;
+    uint32_t value {};  // must be 32 bit long
+    float valueF {};
     for (auto& it : values) {
         str >> value;
         it.ambientColor.setPackedValue(value);
@@ -3153,8 +3161,8 @@ void PropertyMaterialList::RestoreDocFileV1(Base::Reader& reader)
     uint32_t count = 0;
     str >> count;
     std::vector<Material> values(count);
-    uint32_t value;  // must be 32 bit long
-    float valueF;
+    uint32_t value {};  // must be 32 bit long
+    float valueF {};
     for (auto& it : values) {
         str >> value;
         it.ambientColor.setPackedValue(value);
diff --git a/src/App/PropertyStandard.h b/src/App/PropertyStandard.h
index ed617cc821..60978ecb53 100644
--- a/src/App/PropertyStandard.h
+++ b/src/App/PropertyStandard.h
@@ -1189,13 +1189,21 @@ public:
 protected:
     Material getPyValue(PyObject* py) const override;
 
+private:
+    enum Format {
+        Version_0,
+        Version_1,
+        Version_2
+    };
+
     void RestoreDocFileV0(uint32_t count, Base::Reader& reader);
     void RestoreDocFileV1(Base::Reader& reader);
 
-private:
     void verifyIndex(int index) const;
     void setSizeOne();
     int resizeByOneIfNeeded(int index);
+
+    Format formatVersion {Version_0};
 };