What is the best way to fix "Improper Restriction of xml external entity reference"?

Question

We recently run VeraCode that points out on the following method:

    public XmlElement RunProcedureXmlElement(string Procedure, List<SqlParameter> Parameters)
    {
        DataSet ds = RunProcedureDataSet(Procedure, Parameters);
        XmlDocument xmlDoc = new XmlDocument();
        StringBuilder strXML = new StringBuilder();

        foreach (DataTable dt in ds.Tables)
        {
            foreach (DataRow dr in dt.Rows)
            {
                strXML.Append(dr[0]); // Do I still need .ToString()???
            }
        }
        if (strXML.Length == 0) strXML.Append("<root total=\"0\"></root>");

        try
        {
            xmlDoc.LoadXml(strXML.ToString());
        }
        catch (XmlException e)
        {

        }

        return xmlDoc.DocumentElement;
    }

What would be a good solution to fix that method so VeraCode stops complaining?

Thank's

This is C#, right? Would be helpful to tag appropriately, since fixing this (by disallowing the document from providing its own DTD) requires knowledge of the platform's XML APIs. — Charles Duffy, Feb 21 '14 at 15:17
Ahh - I see you already did prohibit DTD processing. Sounds like a bug in VeraCode, then; I'd take it to them. — Charles Duffy, Feb 21 '14 at 15:18
I put DtdProcessing.Prohibit in order to fix it. I did not have it before. So, no problem from VeraCode. VeraCode pointed to the line where my method begins. I was wondering if you know which line in this method VeraCode could complain about? Is it where I append "" and if yes can I replace it? Or it just complained because I did not take care of TD processing? — piterskiy, Feb 21 '14 at 15:23
As a customer, you'd have access to documentation as good as anyone else outside the company -- but if _I_ were writing a static analyzer looking for this particular issue, it would be the `Load()` and `LoadXml()` calls that I'd look for to trigger this particular analysis. — Charles Duffy, Feb 21 '14 at 15:30
Thank's Charles. So, basically, putting DtdProcessing.Prohibit is enough to take care of this issue? — piterskiy, Feb 21 '14 at 15:33
Yes. You might take your fix out of the question and make it an answer. :) — Charles Duffy, Feb 21 '14 at 15:36

score 6 · Answer 1 · edited Dec 17 '15 at 19:37

6

I also had the same issue with Veracode, and the following resolved it.
After declaring XmlReader:

XmlDocument xmlDoc = new XmlDocument();

Add line:

xmlDoc.XmlResolver = null;

edited Dec 17 '15 at 19:37

kvorobiev

5,012
4
29
35

answered Dec 17 '15 at 18:37

David Grigorian

84
1
4

score 3 · Answer 2 · answered Feb 24 '14 at 14:40

After doing some research, this piece of code should fix it:

        using (System.IO.MemoryStream stream = new System.IO.MemoryStream (Encoding.Default.GetBytes(strXML.ToString())))
        {
            XmlReaderSettings settings = new XmlReaderSettings();
            settings.DtdProcessing = DtdProcessing.Prohibit;
            using (XmlReader reader = XmlReader.Create(stream, settings))
            {
                try
                {
                    xmlDoc.Load(reader);
                }
                catch(XmlException e)
                {

                }
            }
        }

score 2 · Answer 3 · edited Jun 08 '18 at 09:03

2

I used following example to solve this issues

  XmlDocument xmlDoc = new XmlDocument();
  xmlDoc.XmlResolver = null;
  xmlDoc.LoadXml(strXML.ToString());

edited Jun 08 '18 at 09:03

LuFFy

8,799
10
41
59

answered Jun 08 '18 at 08:41

Pramod Gadekar

31
4

score 2 · Answer 4 · edited Sep 10 '20 at 09:18

2

From VS2017 IDE advice, you could correct it by this :

    XmlDocument xmlDoc = new XmlDocument { XmlResolver = null };

edited Sep 10 '20 at 09:18

F.H.

1,456
1
20
34

answered Sep 10 '20 at 08:53

Alex Hsu

21
5

Finally, I solve this issue with David 's solution. Cause the Semi-Trusted software no support. – Alex Hsu Sep 17 '20 at 01:53

What is the best way to fix "Improper Restriction of xml external entity reference"?

4 Answers4

Linked

Related