monogame-0001/0002/0003: MonoGame CWE-407 scan, 3 defects

monogame-0001: IntermediateWriter.WriteSharedResources writtenSharedResources
  List<string>.Contains in while loop O(R^2), fix: HashSet<string> MEDIUM
monogame-0002: IntermediateSerializer._scannedObjects List<object>.Contains
  per object during scan O(N^2), fix: HashSet<object> MEDIUM
monogame-0003: OpenAssetImporter._bones List<Node>.Contains in recursive
  tree import O(N*B), fix: HashSet<Node> MEDIUM
MOAD-0002 through 0005: CLEAN (single-threaded game framework, no secrets,
  no leaked context, no thundering herd)

defects/monogame-0001/patch/monogame-0001.patch

@@ -0,0 +1,22 @@
+--- a/MonoGame.Framework.Content.Pipeline/Serialization/Intermediate/IntermediateWriter.cs
++++ b/MonoGame.Framework.Content.Pipeline/Serialization/Intermediate/IntermediateWriter.cs
+@@ -217,13 +217,13 @@ namespace Microsoft.Xna.Framework.Content.Pipeline.Serialization.Intermediate
+         internal void WriteSharedResources()
+         {
+-            if (!_sharedResources.Any())
++            if (_sharedResources.Count == 0)
+                 return;
+
+             Xml.WriteStartElement("Resources");
+
+             // Loop like this because we might create more shared resources while we're serializing.
+-            var writtenSharedResources = new List<string>();
+-            while (_sharedResources.Any(x => !writtenSharedResources.Contains(x.Value)))
++            var writtenSharedResources = new HashSet<string>();
++            while (_sharedResources.Any(x => !writtenSharedResources.Contains(x.Value)))
+             {
+-                var sharedResource = _sharedResources.First(x => !writtenSharedResources.Contains(x.Value));
++                var sharedResource = _sharedResources.First(x => !writtenSharedResources.Contains(x.Value));
+                 writtenSharedResources.Add(sharedResource.Value);
+
+                 WriteSharedResource(sharedResource.Value, sharedResource.Key);

defects/monogame-0001/test/monogame-0001-test.cs

@@ -0,0 +1,81 @@
+// monogame-0001-test: IntermediateWriter.WriteSharedResources List<string>.Contains O(R^2)
+// The writtenSharedResources list uses List<string>.Contains() inside a while loop
+// that iterates through all shared resources. With a HashSet, Contains is O(1).
+//
+// Defect: IntermediateWriter.cs line 226
+//   var writtenSharedResources = new List<string>();
+//   while (_sharedResources.Any(x => !writtenSharedResources.Contains(x.Value)))
+//
+// Fix: HashSet<string> for O(1) lookup instead of O(N) list scan.
+
+using System;
+using System.Collections.Generic;
+using System.Diagnostics;
+using System.Linq;
+
+public class MonoGame0001Test
+{
+    // Simulate the defective pattern: List<string>.Contains in a while loop
+    static long SimulateDefective(int resourceCount)
+    {
+        var sharedResources = new Dictionary<object, string>();
+        for (int i = 0; i < resourceCount; i++)
+            sharedResources[new object()] = "#Resource" + (i + 1);
+
+        var sw = Stopwatch.StartNew();
+        var writtenSharedResources = new List<string>();
+        while (sharedResources.Any(x => !writtenSharedResources.Contains(x.Value)))
+        {
+            var sharedResource = sharedResources.First(x => !writtenSharedResources.Contains(x.Value));
+            writtenSharedResources.Add(sharedResource.Value);
+        }
+        sw.Stop();
+        return sw.ElapsedTicks;
+    }
+
+    // Simulate the fixed pattern: HashSet<string>.Contains in a while loop
+    static long SimulateFixed(int resourceCount)
+    {
+        var sharedResources = new Dictionary<object, string>();
+        for (int i = 0; i < resourceCount; i++)
+            sharedResources[new object()] = "#Resource" + (i + 1);
+
+        var sw = Stopwatch.StartNew();
+        var writtenSharedResources = new HashSet<string>();
+        while (sharedResources.Any(x => !writtenSharedResources.Contains(x.Value)))
+        {
+            var sharedResource = sharedResources.First(x => !writtenSharedResources.Contains(x.Value));
+            writtenSharedResources.Add(sharedResource.Value);
+        }
+        sw.Stop();
+        return sw.ElapsedTicks;
+    }
+
+    static void Main()
+    {
+        // Warmup
+        SimulateDefective(10);
+        SimulateFixed(10);
+
+        int N = 2000;
+        int trials = 3;
+
+        long defectiveTotal = 0;
+        long fixedTotal = 0;
+
+        for (int t = 0; t < trials; t++)
+        {
+            defectiveTotal += SimulateDefective(N);
+            fixedTotal += SimulateFixed(N);
+        }
+
+        double ratio = (double)defectiveTotal / fixedTotal;
+
+        Console.WriteLine($"monogame-0001: IntermediateWriter writtenSharedResources");
+        Console.WriteLine($"  N={N} shared resources, {trials} trials");
+        Console.WriteLine($"  Defective (List):    {defectiveTotal / trials} ticks avg");
+        Console.WriteLine($"  Fixed (HashSet):     {fixedTotal / trials} ticks avg");
+        Console.WriteLine($"  Ratio: {ratio:F1}x");
+        Console.WriteLine(ratio > 2.0 ? "  PASS: defect confirmed" : "  FAIL: ratio too low");
+    }
+}

defects/monogame-0002/patch/monogame-0002.patch

@@ -0,0 +1,23 @@
+--- a/MonoGame.Framework.Content.Pipeline/Serialization/Intermediate/IntermediateSerializer.cs
++++ b/MonoGame.Framework.Content.Pipeline/Serialization/Intermediate/IntermediateSerializer.cs
+@@ -59,7 +59,7 @@ namespace Microsoft.Xna.Framework.Content.Pipeline.Serialization.Intermediate
+         private IntermediateSerializer()
+         {
+-            _scannedObjects = new List<object>();
++            _scannedObjects = new HashSet<object>(ReferenceEqualityComparer.Instance);
+             _namespaceAliasHelper = new NamespaceAliasHelper(this);
+         }
+
+@@ -75,9 +75,9 @@ namespace Microsoft.Xna.Framework.Content.Pipeline.Serialization.Intermediate
+
+-        private readonly List<object> _scannedObjects;
++        private readonly HashSet<object> _scannedObjects;
+
+         internal bool AlreadyScanned(object value)
+         {
+-            if (_scannedObjects.Contains(value))
+-                return true;
+-            _scannedObjects.Add(value);
+-            return false;
++            return !_scannedObjects.Add(value);
+         }

defects/monogame-0002/test/monogame-0002-test.cs

@@ -0,0 +1,81 @@
+// monogame-0002-test: IntermediateSerializer._scannedObjects List<object>.Contains O(N^2)
+// Every object scanned during serialization does a List<object>.Contains check,
+// which is O(N) per call, making the total scan O(N^2).
+//
+// Defect: IntermediateSerializer.cs line 375
+//   private readonly List<object> _scannedObjects;
+//   if (_scannedObjects.Contains(value)) return true;
+//   _scannedObjects.Add(value);
+//
+// Fix: HashSet<object> with ReferenceEqualityComparer for O(1) lookup.
+
+using System;
+using System.Collections.Generic;
+using System.Diagnostics;
+
+public class MonoGame0002Test
+{
+    // Simulate the defective pattern
+    static long SimulateDefective(int objectCount)
+    {
+        var scannedObjects = new List<object>();
+        var objects = new object[objectCount];
+        for (int i = 0; i < objectCount; i++)
+            objects[i] = new object();
+
+        var sw = Stopwatch.StartNew();
+        foreach (var obj in objects)
+        {
+            if (scannedObjects.Contains(obj))
+                continue;
+            scannedObjects.Add(obj);
+        }
+        sw.Stop();
+        return sw.ElapsedTicks;
+    }
+
+    // Simulate the fixed pattern
+    static long SimulateFixed(int objectCount)
+    {
+        var scannedObjects = new HashSet<object>(ReferenceEqualityComparer.Instance);
+        var objects = new object[objectCount];
+        for (int i = 0; i < objectCount; i++)
+            objects[i] = new object();
+
+        var sw = Stopwatch.StartNew();
+        foreach (var obj in objects)
+        {
+            scannedObjects.Add(obj);
+        }
+        sw.Stop();
+        return sw.ElapsedTicks;
+    }
+
+    static void Main()
+    {
+        // Warmup
+        SimulateDefective(100);
+        SimulateFixed(100);
+
+        int N = 10000;
+        int trials = 3;
+
+        long defectiveTotal = 0;
+        long fixedTotal = 0;
+
+        for (int t = 0; t < trials; t++)
+        {
+            defectiveTotal += SimulateDefective(N);
+            fixedTotal += SimulateFixed(N);
+        }
+
+        double ratio = (double)defectiveTotal / fixedTotal;
+
+        Console.WriteLine($"monogame-0002: IntermediateSerializer _scannedObjects");
+        Console.WriteLine($"  N={N} objects, {trials} trials");
+        Console.WriteLine($"  Defective (List):    {defectiveTotal / trials} ticks avg");
+        Console.WriteLine($"  Fixed (HashSet):     {fixedTotal / trials} ticks avg");
+        Console.WriteLine($"  Ratio: {ratio:F1}x");
+        Console.WriteLine(ratio > 2.0 ? "  PASS: defect confirmed" : "  FAIL: ratio too low");
+    }
+}

defects/monogame-0003/patch/monogame-0003.patch

@@ -0,0 +1,24 @@
+--- a/MonoGame.Framework.Content.Pipeline/OpenAssetImporter.cs
++++ b/MonoGame.Framework.Content.Pipeline/OpenAssetImporter.cs
+@@ -217,7 +217,7 @@ namespace Microsoft.Xna.Framework.Content.Pipeline
+         private Dictionary<string, Matrix> _deformationBones;   // The names and offset matrices of all deformation bones.
+         private Node _rootBone;                                 // The node that represents the root bone.
+-        private List<Node> _bones = new List<Node>();           // All nodes attached to the root bone.
++        private HashSet<Node> _bones = new HashSet<Node>();     // All nodes attached to the root bone.
+         private Dictionary<string, FbxPivot> _pivots;              // The transformation pivots.
+
+         // XNA content
+@@ -1115,7 +1115,7 @@ namespace Microsoft.Xna.Framework.Content.Pipeline
+-        private static void GetSubtree(Node node, List<Node> list)
++        private static void GetSubtree(Node node, ICollection<Node> collection)
+         {
+             Debug.Assert(node != null);
+-            Debug.Assert(list != null);
++            Debug.Assert(collection != null);
+
+-            list.Add(node);
++            collection.Add(node);
+             foreach (var child in node.Children)
+-                GetSubtree(child, list);
++                GetSubtree(child, collection);
+         }

defects/monogame-0003/test/monogame-0003-test.cs

@@ -0,0 +1,116 @@
+// monogame-0003-test: OpenAssetImporter._bones List<Node>.Contains O(N*B)
+// During recursive node import, _bones.Contains(aiNode) is called for every
+// non-mesh, non-pivot node. With _bones as a List<Node>, this is O(B) per call,
+// making the full tree traversal O(N*B) where N=total nodes, B=bone count.
+//
+// Defect: OpenAssetImporter.cs line 583, 818
+//   private List<Node> _bones = new List<Node>();
+//   else if (!_bones.Contains(aiNode))  // O(B) per node
+//
+// Fix: HashSet<Node> for O(1) Contains.
+
+using System;
+using System.Collections.Generic;
+using System.Diagnostics;
+
+public class MonoGame0003Test
+{
+    class FakeNode
+    {
+        public string Name;
+        public List<FakeNode> Children = new List<FakeNode>();
+    }
+
+    // Simulate the defective pattern: List.Contains in tree traversal
+    static long SimulateDefective(FakeNode root, List<FakeNode> bones)
+    {
+        var sw = Stopwatch.StartNew();
+        TraverseDefective(root, bones);
+        sw.Stop();
+        return sw.ElapsedTicks;
+    }
+
+    static void TraverseDefective(FakeNode node, List<FakeNode> bones)
+    {
+        // Simulates the check at line 583
+        if (!bones.Contains(node))
+        {
+            // would create NodeContent
+        }
+        foreach (var child in node.Children)
+            TraverseDefective(child, bones);
+    }
+
+    // Simulate the fixed pattern: HashSet.Contains in tree traversal
+    static long SimulateFixed(FakeNode root, HashSet<FakeNode> bones)
+    {
+        var sw = Stopwatch.StartNew();
+        TraverseFixed(root, bones);
+        sw.Stop();
+        return sw.ElapsedTicks;
+    }
+
+    static void TraverseFixed(FakeNode node, HashSet<FakeNode> bones)
+    {
+        if (!bones.Contains(node))
+        {
+            // would create NodeContent
+        }
+        foreach (var child in node.Children)
+            TraverseFixed(child, bones);
+    }
+
+    static FakeNode BuildTree(int depth, int breadth, List<FakeNode> allNodes)
+    {
+        var node = new FakeNode { Name = "node_" + allNodes.Count };
+        allNodes.Add(node);
+        if (depth > 0)
+        {
+            for (int i = 0; i < breadth; i++)
+            {
+                var child = BuildTree(depth - 1, breadth, allNodes);
+                node.Children.Add(child);
+            }
+        }
+        return node;
+    }
+
+    static void Main()
+    {
+        // Build a tree with ~1000 nodes (depth=3, breadth=10 = 1111 nodes)
+        var allNodes = new List<FakeNode>();
+        var root = BuildTree(3, 10, allNodes);
+
+        // Half the nodes are bones
+        var bonesList = new List<FakeNode>();
+        var bonesSet = new HashSet<FakeNode>();
+        for (int i = 0; i < allNodes.Count; i += 2)
+        {
+            bonesList.Add(allNodes[i]);
+            bonesSet.Add(allNodes[i]);
+        }
+
+        // Warmup
+        SimulateDefective(root, bonesList);
+        SimulateFixed(root, bonesSet);
+
+        int trials = 5;
+        long defectiveTotal = 0;
+        long fixedTotal = 0;
+
+        for (int t = 0; t < trials; t++)
+        {
+            defectiveTotal += SimulateDefective(root, bonesList);
+            fixedTotal += SimulateFixed(root, bonesSet);
+        }
+
+        double ratio = (double)defectiveTotal / fixedTotal;
+
+        Console.WriteLine($"monogame-0003: OpenAssetImporter _bones");
+        Console.WriteLine($"  Nodes={allNodes.Count}, Bones={bonesList.Count}, {trials} trials");
+        Console.WriteLine($"  Defective (List):    {defectiveTotal / trials} ticks avg");
+        Console.WriteLine($"  Fixed (HashSet):     {fixedTotal / trials} ticks avg");
+        Console.WriteLine($"  Ratio: {ratio:F1}x");
+        Console.WriteLine(ratio > 2.0 ? "  PASS: defect confirmed" : "  FAIL: ratio too low");
+    }
+}