diff --git a/libraries/MTConnect.NET-XML/XsdPreprocessor.cs b/libraries/MTConnect.NET-XML/XsdPreprocessor.cs index 9bdb8b734..1fd717232 100644 --- a/libraries/MTConnect.NET-XML/XsdPreprocessor.cs +++ b/libraries/MTConnect.NET-XML/XsdPreprocessor.cs @@ -41,6 +41,15 @@ public static class XsdPreprocessor { private const string XsdNamespace = "http://www.w3.org/2001/XMLSchema"; + /// + /// Upper bound on the size of an XSD source the preprocessor accepts, in UTF-16 characters + /// (roughly bytes for ASCII XSDs). MTConnect XSDs ship well under 500 KB; the 10 MB gate + /// keeps a bounded ceiling on parser memory and short-circuits XML entity-expansion attacks + /// before the underlying reader allocates. Exposed as a constant so the pinning tests can + /// straddle the boundary without a magic number. + /// + public const int MaxSourceCharacters = 10_000_000; + /// /// Returns a copy of with every XSD /// 1.1-only construct removed. Idempotent: re-running on the @@ -51,23 +60,55 @@ public static class XsdPreprocessor /// /// Thrown when is null. /// + /// + /// Thrown when exceeds + /// — the guard rail against XML entity-expansion attacks. + /// public static string StripXsd11Constructs(string xsdSourceXml) { if (xsdSourceXml == null) throw new ArgumentNullException(nameof(xsdSourceXml)); if (xsdSourceXml.Length == 0) return xsdSourceXml; + // Fail loud before parsing: a caller who lobbed a > 10 MB payload at the preprocessor + // has almost certainly hit an entity-expansion attack (billion-laughs / quadratic- + // blowup) or a runaway build-side generator. Silently returning the raw source would + // hand the payload down to XmlSchema.Read, which lacks a matching size cap. + if (xsdSourceXml.Length > MaxSourceCharacters) + { + throw new System.Xml.XmlException( + $"XSD source exceeds the {MaxSourceCharacters:N0}-character preprocessor limit; " + + "refusing to parse to avoid unbounded resource consumption."); + } + XDocument doc; try { - using (var reader = new StringReader(xsdSourceXml)) + // XDocument.Load(TextReader, LoadOptions) uses default XmlReaderSettings, which + // in .NET 4.0+ disables DTD processing by default but leaves DocumentDoS-shape + // limits at their permissive defaults (MaxCharactersInDocument = 0 → unbounded, + // MaxCharactersFromEntities = 0 → unbounded). Route through an explicitly-tightened + // XmlReader so the loader rejects DTDs, refuses to resolve external entities, and + // caps the document at 10 MB / entity expansion at 0 characters to shut down the + // classic billion-laughs and quadratic-blowup XML entity attacks. The XSD source + // is untrusted — an operator can point the preprocessor at any XSD URL — so the + // hardening runs at every entry. + var settings = new System.Xml.XmlReaderSettings + { + DtdProcessing = System.Xml.DtdProcessing.Prohibit, + XmlResolver = null, + MaxCharactersInDocument = 10_000_000, + MaxCharactersFromEntities = 0, + }; + using (var stringReader = new StringReader(xsdSourceXml)) + using (var xmlReader = System.Xml.XmlReader.Create(stringReader, settings)) { - doc = XDocument.Load(reader, LoadOptions.PreserveWhitespace); + doc = XDocument.Load(xmlReader, LoadOptions.PreserveWhitespace); } } catch (System.Xml.XmlException) { - // Not well-formed XML — let the downstream BCL reader emit - // the parse error so the caller's error path stays + // Not well-formed XML, or exceeds the DTD/size guard rails above — let the + // downstream BCL reader emit the parse error so the caller's error path stays // consistent. return xsdSourceXml; } diff --git a/tests/MTConnect.NET-XML-Tests/Xml/XsdPreprocessorSecurityTests.cs b/tests/MTConnect.NET-XML-Tests/Xml/XsdPreprocessorSecurityTests.cs new file mode 100644 index 000000000..48db41062 --- /dev/null +++ b/tests/MTConnect.NET-XML-Tests/Xml/XsdPreprocessorSecurityTests.cs @@ -0,0 +1,99 @@ +// Copyright (c) 2026 TrakHound Inc., All Rights Reserved. +// TrakHound Inc. licenses this file to you under the MIT license. + +using System.Text; +using System.Xml; +using NUnit.Framework; + +namespace MTConnect.Xml.Tests +{ + /// + /// Pins the DoS-hardening guard rails on . + /// The preprocessor loads untrusted XSD text; without the hardening a hostile source can trigger + /// the classic XML entity-expansion attacks (billion-laughs, quadratic blowup) or exhaust memory + /// through a runaway payload. + /// + [TestFixture] + [Category("XsdPreprocessorSecurity")] + public class XsdPreprocessorSecurityTests + { + /// Pins that a source exceeding the character limit raises instead of allocating an unbounded XmlReader. + [Test] + public void StripXsd11Constructs_OversizedInput_RaisesXmlException() + { + // Cheap oversized payload: a single well-formed root element with padding pushing the + // total length just past the guard. The padding lives inside the element so the XML is + // still parseable — the size gate must fire before the parser sees it. + var padding = new string('a', XsdPreprocessor.MaxSourceCharacters); + var oversized = + "" + + $"{padding}" + + ""; + + Assert.That(oversized.Length, Is.GreaterThan(XsdPreprocessor.MaxSourceCharacters), + "the payload must straddle the size gate for the test to be meaningful"); + Assert.Throws(() => XsdPreprocessor.StripXsd11Constructs(oversized)); + } + + /// Pins that a source referencing an external DTD is rejected — the hardened reader has DtdProcessing set to Prohibit. + [Test] + public void StripXsd11Constructs_ExternalDtdReference_ReturnsSource_Unprocessed() + { + // A well-formed XML with a DOCTYPE declaration. The hardened reader must refuse to + // process the DTD; the preprocessor catches the resulting XmlException and returns the + // raw source (existing not-well-formed contract). + const string withDtd = + "" + + "" + + ""; + + var result = XsdPreprocessor.StripXsd11Constructs(withDtd); + + // Not-well-formed / DTD-prohibited inputs land in the catch and return the raw source + // for the downstream schema reader to error out consistently. + Assert.That(result, Is.EqualTo(withDtd)); + } + + /// Pins that a source with an internal DTD entity expansion is rejected — the hardened reader has DtdProcessing set to Prohibit, so the entity is never expanded. + [Test] + public void StripXsd11Constructs_InternalDtdEntityExpansion_ReturnsSource_Unprocessed() + { + // Simplified billion-laughs shape: three levels of entity expansion. Any bytes-in-entities + // pass would blow up the parser; the DTD prohibition means the parser never even sees + // the definitions. + var sb = new StringBuilder(); + sb.Append(""); + sb.Append(""); + sb.Append(""); + sb.Append(""); + sb.Append("]>"); + sb.Append("&c;"); + var laugh = sb.ToString(); + + var result = XsdPreprocessor.StripXsd11Constructs(laugh); + + Assert.That(result, Is.EqualTo(laugh), + "the DTD-prohibited reader must refuse the entity-expansion payload and the " + + "preprocessor must return the raw source unprocessed"); + } + + /// Pins that a well-formed, small XSD still round-trips through the hardened loader. + [Test] + public void StripXsd11Constructs_SmallWellFormedXsd_RoundTripsThroughHardenedLoader() + { + // Regression guard: the hardening MUST NOT break the happy path for the shipped + // MTConnect XSDs, which are well under the size cap and carry no DTD. + const string xsd = + "" + + "" + + ""; + + var result = XsdPreprocessor.StripXsd11Constructs(xsd); + + Assert.That(result, Is.Not.Null.And.Not.Empty); + Assert.That(result, Does.Contain("