Validate tree depth in XML parsing (AcademySoftwareFoundation#2232)

This changelist adds validation for the tree depth in XML parsing, preventing an invalid document from triggering a stack overflow.

A new unit test has been added to parse a document exceeding the maximum tree depth, verifying that the correct exception is thrown.

Author: jstone-lucasfilm

source/MaterialXFormat/XmlIo.cpp

@@ -19,14 +19,16 @@ MATERIALX_NAMESPACE_BEGIN
 
 const string MTLX_EXTENSION = "mtlx";
 
+const int MAX_XML_TREE_DEPTH = 256;
+
 namespace
 {
 
 const string XINCLUDE_TAG = "xi:include";
 const string XINCLUDE_NAMESPACE = "xmlns:xi";
 const string XINCLUDE_URL = "http://www.w3.org/2001/XInclude";
 
-void elementFromXml(const xml_node& xmlNode, ElementPtr elem, const XmlReadOptions* readOptions)
+void elementFromXml(const xml_node& xmlNode, ElementPtr elem, const XmlReadOptions* readOptions, int depth = 1)
 {
     // Store attributes in element.
     for (const xml_attribute& xmlAttr : xmlNode.attributes())
@@ -58,9 +60,15 @@ void elementFromXml(const xml_node& xmlNode, ElementPtr elem, const XmlReadOptio
             continue;
         }
 
+        // Enforce maximum tree depth.
+        if (depth >= MAX_XML_TREE_DEPTH)
+        {
+            throw ExceptionParseError("Maximum tree depth exceeded.");
+        }
+
         // Create the new element.
         ElementPtr child = elem->addChildOfCategory(category, name);
-        elementFromXml(xmlChild, child, readOptions);
+        elementFromXml(xmlChild, child, readOptions, depth + 1);
 
         // Handle the interpretation of XML comments and newlines.
         if (readOptions && category.empty())

source/MaterialXFormat/XmlIo.h

@@ -22,6 +22,8 @@ class XmlReadOptions;
 
 extern MX_FORMAT_API const string MTLX_EXTENSION;
 
+extern MX_FORMAT_API const int MAX_XML_TREE_DEPTH;
+
 /// A standard function that reads from an XML file into a Document, with
 /// optional search path and read options.
 using XmlReadFunction = std::function<void(DocumentPtr, const FilePath&, const FileSearchPath&, const XmlReadOptions*)>;

source/MaterialXTest/MaterialXFormat/XmlIo.cpp

@@ -262,6 +262,25 @@ TEST_CASE("Comments and newlines", "[xmlio]")
     REQUIRE(origXml == newXml);
 }
 
+TEST_CASE("Maximum tree depth", "[xmlio]")
+{
+    // Create a document that exceeds the maximum tree depth.
+    mx::DocumentPtr doc = mx::createDocument();
+    mx::ElementPtr elem = doc;
+    for (int i = 0; i < mx::MAX_XML_TREE_DEPTH + 1; i++)
+    {
+        elem = elem->addChild<mx::NodeGraph>();
+    }
+
+    // Write the document to a string buffer.
+    std::string xmlString = mx::writeToXmlString(doc);
+
+    // Read the string buffer as a document, verifying that the correct
+    // exception is thrown.
+    mx::DocumentPtr newDoc = mx::createDocument();
+    REQUIRE_THROWS_AS(mx::readFromXmlString(newDoc, xmlString), mx::ExceptionParseError);
+}
+
 TEST_CASE("Fuzz testing", "[xmlio]")
 {
     mx::FileSearchPath searchPath = mx::getDefaultDataSearchPath();