Skip to content

Commit cddeda2

Browse files
committed
fix(xml): harden VendorExtensions XElement parse against XXE
Explicitly pin DtdProcessing.Prohibit and XmlResolver = null when parsing each captured vendor-extension element's OuterXml back into an XElement, mirroring the repo-wide defense-in-depth convention already established in XmiDeserializer.FromXml/FromFile. .NET 6+ already defaults XmlResolver to null and disables DTD processing on the outer document read, and a DOCTYPE cannot legally appear inside a captured element's OuterXml, so this closes a consistency gap rather than an exploitable path today — but it survives a future framework downgrade or accidental restoration of XmlUrlResolver, and keeps every XML parse entry point in the repo on the same explicit hardening idiom. Found by the security-audit pass of the PR TrakHound#223 dime review cycle.
1 parent 4af3f25 commit cddeda2

1 file changed

Lines changed: 19 additions & 2 deletions

File tree

libraries/MTConnect.NET-XML/Devices/XmlConfiguration.cs

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
using MTConnect.Devices.Configurations;
55
using System.Collections.Generic;
6+
using System.IO;
67
using System.Linq;
78
using System.Xml;
89
using System.Xml.Linq;
@@ -146,7 +147,7 @@ public IConfiguration ToConfiguration()
146147
configuration.Specifications = specifications;
147148
}
148149

149-
// Vendor Extensions — every unrecognised child element captured by
150+
// Vendor Extensions — every unrecognized child element captured by
150151
// [XmlAnyElement] projects to an XElement on the model. Elements are
151152
// preserved verbatim so downstream consumers see the exact
152153
// vendor-namespaced XML the operator authored.
@@ -157,7 +158,23 @@ public IConfiguration ToConfiguration()
157158
{
158159
if (element != null)
159160
{
160-
extensions.Add(XElement.Parse(element.OuterXml, LoadOptions.PreserveWhitespace));
161+
// Defense-in-depth against XML External Entity (XXE)
162+
// attacks: .NET 6+ already defaults XmlResolver to
163+
// null and disables DTD processing on the outer
164+
// document read (and a DOCTYPE cannot legally appear
165+
// inside a captured element's OuterXml), but pinning
166+
// both explicitly here mirrors the repo-wide
167+
// convention (see XmiDeserializer.FromXml) and
168+
// survives a future framework downgrade or
169+
// accidental restoration of XmlUrlResolver.
170+
var settings = new XmlReaderSettings
171+
{
172+
DtdProcessing = DtdProcessing.Prohibit,
173+
XmlResolver = null,
174+
};
175+
using var stringReader = new StringReader(element.OuterXml);
176+
using var xmlReader = XmlReader.Create(stringReader, settings);
177+
extensions.Add(XElement.Load(xmlReader, LoadOptions.PreserveWhitespace));
161178
}
162179
}
163180

0 commit comments

Comments
 (0)