So I am running into an issue when I run a security scan on my application. It turns out that I am failing to protect against XXE. Here is a short snippet showing the offending code:
static void Main()
{
string inp = Console.ReadLine();
string xmlStr = ""; //This has a value that is much too long to put into a single post
if (!string.IsNullOrEmpty(inp))
{
xmlStr = inp;
}
XmlDocument xmlDocObj = new XmlDocument {XmlResolver = null};
xmlDocObj.LoadXml(xmlStr);
XmlNodeList measureXmlNodeListObj = xmlDocObj.SelectNodes("REQ/MS/M");
foreach (XmlNode measureXmlNodeObj in measureXmlNodeListObj)
{
XmlNode detailXmlNodeListObj = xmlDocObj.SelectSingleNode("REQ/DTD");
string measureKey = measureXmlNodeObj.Attributes["KY"].Value;
if (detailXmlNodeListObj.Attributes["MKY"].Value ==
measureKey) //Checking if selected MeasureKey is same
{
XmlNode filerNode = measureXmlNodeObj.SelectSingleNode("FS");
if (filerNode != null)
{
XDocument fixedFilterXmlObj = XDocument.Load(new StringReader(filerNode.OuterXml));
var measureFixedFilters = (from m in fixedFilterXmlObj.Element("FS").Elements("F")
select m).ToList();
foreach (var fixedFilter in measureFixedFilters)
{
var fixedFilterValues = (from m in fixedFilter.Elements("VS").Elements("V")
select m.Attribute("DESC").Value).ToList();
foreach (var value in fixedFilterValues)
{
Console.WriteLine(value.Trim());
}
}
}
}
}
Console.ReadLine();
}
According to Veracode, the line that unsafe is XDocument fixedFilterXmlObj = XDocument.Load(new StringReader(filerNode.OuterXml));
But it seems like according to Owsap, it should be safe:
Both the XElement and XDocument objects in the System.Xml.Linq library are safe from XXE injection by default. XElement parses only the elements within the XML file, so DTDs are ignored altogether. XDocument has DTDs disabled by default, and is only unsafe if constructed with a different unsafe XML parser.
So it seems like I am making the mistake of using an usafe XML Parser, opening XDocument
to XXE.
I found a unit test that replicates the issue and also has a safe usage of XDocument
but I can't seem to find what exactly my code is unsafe, because I do not use:
XmlReaderSettings settings = new XmlReaderSettings();
settings.DtdProcessing = DtdProcessing.Parse; // unsafe!
You can run my code to replicate the issue, but you should replace the line with the empty xmlStr with this value: here (too large for a single post)
I'm not sure how or why this works, but it does:
XDocument fixedFilterXmlObj;
using (XmlNodeReader nodeReader = new XmlNodeReader(filerNode))
{
nodeReader.MoveToContent();
fixedFilterXmlObj = XDocument.Load(nodeReader);
}