From 1bc7c710d645a55f488857614c7c8d08eb6a137d Mon Sep 17 00:00:00 2001
From: Max Vollmer <mavollme@microsoft.com>
Date: Wed, 11 Mar 2020 09:54:24 +0000
Subject: [PATCH 1/2] Added a check to detect and prevent recursive references
 in GLTF2 files

---
 code/glTF2/glTF2Asset.h   | 7 +++++++
 code/glTF2/glTF2Asset.inl | 9 ++++++++-
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/code/glTF2/glTF2Asset.h b/code/glTF2/glTF2Asset.h
index d3c1654d0..171400368 100644
--- a/code/glTF2/glTF2Asset.h
+++ b/code/glTF2/glTF2Asset.h
@@ -56,6 +56,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 #include <assimp/Exceptional.h>
 
 #include <map>
+#include <set>
 #include <string>
 #include <list>
 #include <vector>
@@ -82,14 +83,18 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 #       define ASSIMP_GLTF_USE_UNORDERED_MULTIMAP
 #   else
 #       define gltf_unordered_map map
+#       define gltf_unordered_set set
 #endif
 
 #ifdef ASSIMP_GLTF_USE_UNORDERED_MULTIMAP
 #   include <unordered_map>
+#   include <unordered_set>
 #   if _MSC_VER > 1600
 #       define gltf_unordered_map unordered_map
+#       define gltf_unordered_set unordered_set
 #   else
 #       define gltf_unordered_map tr1::unordered_map
+#       define gltf_unordered_set tr1::unordered_set
 #   endif
 #endif
 
@@ -942,6 +947,8 @@ namespace glTF2
         Value*              mDict;         //! JSON dictionary object
         Asset&              mAsset;        //! The asset instance
 
+        std::gltf_unordered_set<unsigned int> mRecursiveReferenceCheck;  //! Used by Retrieve to prevent recursive lookups
+
         void AttachToDocument(Document& doc);
         void DetachFromDocument();
 
diff --git a/code/glTF2/glTF2Asset.inl b/code/glTF2/glTF2Asset.inl
index 35ecfa62d..270020aef 100644
--- a/code/glTF2/glTF2Asset.inl
+++ b/code/glTF2/glTF2Asset.inl
@@ -270,6 +270,11 @@ Ref<T> LazyDict<T>::Retrieve(unsigned int i)
         throw DeadlyImportError("GLTF: Object at index \"" + to_string(i) + "\" is not a JSON object");
     }
 
+    if (mRecursiveReferenceCheck.find(i) != mRecursiveReferenceCheck.end()) {
+        throw DeadlyImportError("GLTF: Object at index \"" + to_string(i) + "\" has recursive reference to itself");
+    }
+    mRecursiveReferenceCheck.insert(i);
+
     // Unique ptr prevents memory leak in case of Read throws an exception
     auto inst = std::unique_ptr<T>(new T());
     inst->id = std::string(mDictId) + "_" + to_string(i);
@@ -277,7 +282,9 @@ Ref<T> LazyDict<T>::Retrieve(unsigned int i)
     ReadMember(obj, "name", inst->name);
     inst->Read(obj, mAsset);
 
-    return Add(inst.release());
+    Ref<T> result = Add(inst.release());
+    mRecursiveReferenceCheck.erase(i);
+    return result;
 }
 
 template<class T>

From ec69e2bf59014e60f4297d86ff6cb27f90420e97 Mon Sep 17 00:00:00 2001
From: Max Vollmer <mavollme@microsoft.com>
Date: Wed, 11 Mar 2020 15:32:28 +0000
Subject: [PATCH 2/2] Added unit test for recursive references in GLTF2 file

---
 .../glTF2/RecursiveNodes/RecursiveNodes.gltf  | 25 +++++++++++++++++++
 test/unit/utglTF2ImportExport.cpp             |  6 +++++
 2 files changed, 31 insertions(+)
 create mode 100644 test/models/glTF2/RecursiveNodes/RecursiveNodes.gltf

diff --git a/test/models/glTF2/RecursiveNodes/RecursiveNodes.gltf b/test/models/glTF2/RecursiveNodes/RecursiveNodes.gltf
new file mode 100644
index 000000000..11cf04166
--- /dev/null
+++ b/test/models/glTF2/RecursiveNodes/RecursiveNodes.gltf
@@ -0,0 +1,25 @@
+{
+    "asset": {
+        "version": "2.0"
+    },
+    "scene": 0,
+    "scenes": [
+        {
+            "nodes": [
+                0
+            ]
+        }
+    ],
+    "nodes": [
+        {
+            "children": [
+                1
+            ]
+        },
+        {
+            "children": [
+                0
+            ]
+        }
+    ]
+}
diff --git a/test/unit/utglTF2ImportExport.cpp b/test/unit/utglTF2ImportExport.cpp
index 8b91577f6..07996306a 100644
--- a/test/unit/utglTF2ImportExport.cpp
+++ b/test/unit/utglTF2ImportExport.cpp
@@ -493,3 +493,9 @@ TEST_F(utglTF2ImportExport, texcoords) {
     EXPECT_EQ(aiGetMaterialInteger(material, AI_MATKEY_GLTF_TEXTURE_TEXCOORD(aiTextureType_UNKNOWN, 0), &uvIndex), aiReturn_SUCCESS);
     EXPECT_EQ(uvIndex, 1);
 }
+
+TEST_F(utglTF2ImportExport, recursive_nodes) {
+    Assimp::Importer importer;
+    const aiScene* scene = importer.ReadFile(ASSIMP_TEST_MODELS_DIR "/glTF2/RecursiveNodes/RecursiveNodes.gltf", aiProcess_ValidateDataStructure);
+    EXPECT_EQ(nullptr, scene);
+}