From 342fa3013bba586329f7f33a815a5e767c14c059 Mon Sep 17 00:00:00 2001
From: Ben Rog-Wilhelm <zorba-github@pavlovian.net>
Date: Tue, 10 Sep 2024 01:26:05 -0500
Subject: [PATCH] Fix: Security issue: XML documents could cause arbitrary HTTP
 requests.

---
 CHANGELOG.md             |  1 +
 src/ReaderXmlDec.cs      | 20 +++-----------------
 src/ReaderXmlRecorder.cs | 10 ++--------
 src/ReaderXmlSimple.cs   | 10 ++--------
 src/UtilXml.cs           | 29 +++++++++++++++++++++++++++++
 5 files changed, 37 insertions(+), 33 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index f67ea803..7a075d08 100644
--- a/CHANGELOG.md
+++ b/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]
diff --git a/src/ReaderXmlDec.cs b/src/ReaderXmlDec.cs
index 5b494a0b..abd30e27 100644
--- a/src/ReaderXmlDec.cs
+++ b/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();
diff --git a/src/ReaderXmlRecorder.cs b/src/ReaderXmlRecorder.cs
index 50cfa266..8fe28c3f 100644
--- a/src/ReaderXmlRecorder.cs
+++ b/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;
             }
 
diff --git a/src/ReaderXmlSimple.cs b/src/ReaderXmlSimple.cs
index 54958329..18333081 100644
--- a/src/ReaderXmlSimple.cs
+++ b/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;
             }
 
diff --git a/src/UtilXml.cs b/src/UtilXml.cs
index 2fc40851..44aa1690 100644
--- a/src/UtilXml.cs
+++ b/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;
+                    }
+                }
+            }
+        }
     }
 }