Our static code analyser reports a security issue on unrestricted document types on a line that does XElement.Parse. The input for that method is the result of some API call.
Acording to the documentation the default for XDocument.Load and XElement.Load is to prohibit DTD processing. Trying to load xml with an (external) DTD results in a run-time exception that states that for security reasons DTD is prohibited in this document.
I can pass the same XML string to XML.Parse. But in that case there is no exception thrown. The documentation suggests that it is implemented on top of the standard XmlReader.Create, so I expected the same exception.
Does XElement.Parse do DTD processing, or does it just ignore the xml declaration and DTD information? Or in other words: can I mark the issue as a false positive, or should I update the code and use an XmlReader and explictly prohibit DTD processing?
var xml = @"<?xml version=""1.0"" standalone=""no""?>
<!DOCTYPE serviceResponse SYSTEM ""serviceResponse.dtd"">
<serviceResponse>
<item>
<name>item 1</name>
</item>
</serviceResponse>";
var stringReader = new StringReader(xml);
var xmlReader = XmlReader.Create(stringReader);
XElement.Load(xmlReader);
throws:
[System.Xml.XmlException: For security reasons DTD is prohibited in this XML document. To enable DTD processing set the DtdProcessing property on XmlReaderSettings to Parse and pass the settings into XmlReader.Create method.]
But this runs without issues:
var xml = @"<?xml version=""1.0"" standalone=""no""?>
<!DOCTYPE serviceResponse SYSTEM ""serviceResponse.dtd"">
<serviceResponse>
<item>
<name>item 1</name>
</item>
</serviceResponse>";
XElement.Parse(xml);
5
It is not documented what XmlReaderSettings the Parse function uses.
but looking at the source code, it calls the GetXmlReaderSettings function, which does the following:
XmlReaderSettings rs = new XmlReaderSettings();
if ((o & LoadOptions.PreserveWhitespace) == 0) rs.IgnoreWhitespace = true;
// DtdProcessing.Parse; Parse is not defined in the public contract
rs.DtdProcessing = (DtdProcessing)2;
rs.MaxCharactersFromEntities = (long)1e7;
So yes, this function does parse DTDs, and therefore may be a security risk according to some. You should pass a custom XmlReader instead.
var xml = @"<?xml version=""1.0"" standalone=""no""?>
<!DOCTYPE serviceResponse SYSTEM ""serviceResponse.dtd"">
<serviceResponse>
<item>
<name>item 1</name>
</item>
</serviceResponse>";
using var reader = XmlReader.Create(new StringReader(xml), yourXmlSettingsHere);
var x = XElement.Load(reader);
dotnetfiddle