From efe02a8675c307a67e9029b1bf6e429d25557495 Mon Sep 17 00:00:00 2001
From: wmayer <wmayer@freecad.org>
Date: Thu, 20 Feb 2025 22:33:04 +0100
Subject: [PATCH] Gui: Fix stackoverflow when loading corrupted file

If an object has a link to itself it may cause a stackoverflow
in several cases:
* If the method claimChildren3D() returns a list containing the
  object of the view provider then Document::handleChildren3D()
  will add a SoGroup to itself as a child. This will result into
  a stackoverflow as soon as an action traverses the scene.
* If the method claimChildren() returns a list containing the
  object of the view provider then DocumentItem::createNewItem()
  causes an infinite loop with DocumentItem::populateItem()

Solution:
* Inside Document::handleChildren3D() avoid to add a SoGroup to itself
* In this specific case fix ViewProviderCoordinateSystem::claimChildren()
  to avoid a cyclic dependency

Hint: Since PR 18126 FreeCAD is vulnerable for this problem.

This fixes issue 19682
---
 src/Gui/Document.cpp                     | 43 ++++++++++++++++++------
 src/Gui/ViewProviderCoordinateSystem.cpp | 13 +++++--
 2 files changed, 42 insertions(+), 14 deletions(-)

diff --git a/src/Gui/Document.cpp b/src/Gui/Document.cpp
index 7182eaae31..e703c41e35 100644
--- a/src/Gui/Document.cpp
+++ b/src/Gui/Document.cpp
@@ -2719,21 +2719,42 @@ void Document::handleChildren3D(ViewProvider* viewProvider, bool deleting)
 
             if(!deleting) {
                 for (const auto & it : children) {
-                    ViewProvider* ChildViewProvider = getViewProvider(it);
-                    if (ChildViewProvider) {
-                        auto itOld = oldChildren.find(static_cast<ViewProviderDocumentObject*>(ChildViewProvider));
+                    if (auto ChildViewProvider = dynamic_cast<ViewProviderDocumentObject*>(getViewProvider(it))) {
+                        auto itOld = oldChildren.find(ChildViewProvider);
                         if(itOld!=oldChildren.end()) oldChildren.erase(itOld);
 
-                        SoSeparator* childRootNode =  ChildViewProvider->getRoot();
-                        childGroup->addChild(childRootNode);
+                        if (SoSeparator* childRootNode =  ChildViewProvider->getRoot()) {
+                            if (childRootNode == childGroup) {
+                                Base::Console().warning("Document::handleChildren3D: Do not add "
+                                                        "group of '%s' to itself\n",
+                                                        it->getNameInDocument());
+                            }
+                            else if (childGroup) {
+                                childGroup->addChild(childRootNode);
+                            }
+                        }
 
-                        SoSeparator* childFrontNode = ChildViewProvider->getFrontRoot();
-                        if (frontGroup && childFrontNode)
-                            frontGroup->addChild(childFrontNode);
+                        if (SoSeparator* childFrontNode = ChildViewProvider->getFrontRoot()) {
+                            if (childFrontNode == frontGroup) {
+                                Base::Console().warning("Document::handleChildren3D: Do not add "
+                                                        "foreground group of '%s' to itself\n",
+                                                        it->getNameInDocument());
+                            }
+                            else if (frontGroup) {
+                                frontGroup->addChild(childFrontNode);
+                            }
+                        }
 
-                        SoSeparator* childBackNode = ChildViewProvider->getBackRoot();
-                        if (backGroup && childBackNode)
-                            backGroup->addChild(childBackNode);
+                        if (SoSeparator* childBackNode = ChildViewProvider->getBackRoot()) {
+                            if (childBackNode == backGroup) {
+                                Base::Console().warning("Document::handleChildren3D: Do not add "
+                                                        "background group of '%s' to itself\n",
+                                                        it->getNameInDocument());
+                            }
+                            else if (backGroup) {
+                                backGroup->addChild(childBackNode);
+                            }
+                        }
 
                         // cycling to all views of the document to remove the viewprovider from the viewer itself
                         for (Gui::BaseView* vIt : d->baseViews) {
diff --git a/src/Gui/ViewProviderCoordinateSystem.cpp b/src/Gui/ViewProviderCoordinateSystem.cpp
index 2689e1031b..3d46ff658f 100644
--- a/src/Gui/ViewProviderCoordinateSystem.cpp
+++ b/src/Gui/ViewProviderCoordinateSystem.cpp
@@ -72,11 +72,18 @@ ViewProviderCoordinateSystem::~ViewProviderCoordinateSystem() {
 
 std::vector<App::DocumentObject*> ViewProviderCoordinateSystem::claimChildren() const
 {
-    return static_cast<App::Origin*>( getObject() )->OriginFeatures.getValues();
+    auto obj = getObject<App::Origin>();
+    std::vector<App::DocumentObject*> childs = obj->OriginFeatures.getValues();
+    auto it = std::find(childs.begin(), childs.end(), obj);
+    if (it != childs.end()) {
+        childs.erase(it);
+    }
+    return childs;
 }
 
-std::vector<App::DocumentObject*> ViewProviderCoordinateSystem::claimChildren3D() const {
-    return claimChildren ();
+std::vector<App::DocumentObject*> ViewProviderCoordinateSystem::claimChildren3D() const
+{
+    return claimChildren();
 }
 
 void ViewProviderCoordinateSystem::attach(App::DocumentObject* pcObject)