Fix: Security issue: XML documents could cause arbitrary HTTP requests.

CHANGELOG.md

@@ -11,6 +11,7 @@ All notable changes to this project will be documented in this file.
 * Type parse cache bug that could cause non-arrays to be incorrectly parsed as arrays.
 * Cloning an object with a factory that depends on a non-shared recorded object can, if the stack depth is too deep, result in the non-shared recorded object not yet being ready.
 * Decs that are also IRecordable cause errors when referenced in other Decs.
+* Security issue: XML documents could cause arbitrary HTTP requests.
 
 
 ## [v0.7.0]

src/ReaderXmlDec.cs

@@ -15,24 +15,10 @@ internal class ReaderFileDecXml : ReaderFileDec
 
         public static ReaderFileDecXml Create(TextReader input, string identifier, Recorder.IUserSettings userSettings)
         {
-            XDocument doc;
-
-            using (var _ = new CultureInfoScope(Config.CultureInfo))
+            XDocument doc = UtilXml.ParseSafely(input);
+            if (doc == null)
             {
-                try
-                {
-                    var settings = new XmlReaderSettings();
-                    settings.IgnoreWhitespace = true;
-                    using (var reader = XmlReader.Create(input, settings))
-                    {
-                        doc = XDocument.Load(reader, LoadOptions.SetLineInfo);
-                    }
-                }
-                catch (System.Xml.XmlException e)
-                {
-                    Dbg.Ex(e);
-                    return null;
-                }
+                return null;
             }
 
             var result = new ReaderFileDecXml();

src/ReaderXmlRecorder.cs

@@ -17,15 +17,9 @@ internal class ReaderFileRecorderXml : ReaderFileRecorder
 
         public static ReaderFileRecorderXml Create(string input, string identifier, Recorder.IUserSettings userSettings)
         {
-            XDocument doc;
-
-            try
-            {
-                doc = XDocument.Parse(input, LoadOptions.SetLineInfo);
-            }
-            catch (System.Xml.XmlException e)
+            XDocument doc = UtilXml.ParseSafely(new System.IO.StringReader(input));
+            if (doc == null)
             {
-                Dbg.Ex(e);
                 return null;
             }
 

src/ReaderXmlSimple.cs

@@ -13,15 +13,9 @@ internal static class ReaderFileXmlSimple
     {
         public static ReaderNodeParseable Create(string input, string rootTag, string identifier, Recorder.IUserSettings userSettings)
         {
-            XDocument doc;
-
-            try
-            {
-                doc = XDocument.Parse(input, LoadOptions.SetLineInfo);
-            }
-            catch (System.Xml.XmlException e)
+            XDocument doc = UtilXml.ParseSafely(new System.IO.StringReader(input));
+            if (doc == null)
             {
-                Dbg.Ex(e);
                 return null;
             }
 

src/UtilXml.cs

@@ -1,3 +1,5 @@
+using System;
+
 namespace Dec
 {
     using System.Collections.Generic;
@@ -61,5 +63,32 @@ internal static string GetText(this XElement element)
             }
             return element.Nodes().OfType<XText>().FirstOrDefault()?.Value;
         }
+
+        internal static XDocument ParseSafely(System.IO.TextReader input)
+        {
+            var settings = new XmlReaderSettings
+            {
+                DtdProcessing = DtdProcessing.Prohibit,
+                XmlResolver = null,
+                IgnoreWhitespace = true,
+            };
+
+            using (var reader = XmlReader.Create(input, settings))
+            {
+                // this may be a second layer of culture info scoping, but I'll live with it for now
+                using (var _ = new CultureInfoScope(Config.CultureInfo))
+                {
+                    try
+                    {
+                        return XDocument.Load(reader, LoadOptions.SetLineInfo);
+                    }
+                    catch (System.Xml.XmlException e)
+                    {
+                        Dbg.Ex(e);
+                        return null;
+                    }
+                }
+            }
+        }
     }
 }