CodeQL queries 1.25
Skip to end of metadata
Go to start of metadata

You are viewing an old version of this page. View the current version.

Compare with Current View Page History

Version 1 Current »

Name: XML injection

Description: Building an XML document from user-controlled sources is vulnerable to insertion of malicious code by the user.

ID: cs/xml-injection

Kind: problem

Severity: error

Query: XMLInjection.ql
/**
 * @name XML injection
 * @description Building an XML document from user-controlled sources is vulnerable to insertion of
 *              malicious code by the user.
 * @kind problem
 * @id cs/xml-injection
 * @problem.severity error
 * @tags security
 *       external/cwe/cwe-091
 */

/*
 * consider: @precision high
 */

import csharp
import semmle.code.csharp.dataflow.flowsources.Remote
import semmle.code.csharp.frameworks.system.Xml

/**
 * A taint-tracking configuration for untrusted user input used in XML.
 */
class TaintTrackingConfiguration extends TaintTracking::Configuration {
  TaintTrackingConfiguration() { this = "XMLInjection" }

  override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }

  override predicate isSink(DataFlow::Node sink) {
    exists(MethodCall mc |
      mc.getTarget().hasName("WriteRaw") and
      mc.getTarget().getDeclaringType().getABaseType*().hasQualifiedName("System.Xml.XmlWriter")
    |
      mc.getArgument(0) = sink.asExpr()
    )
  }

  override predicate isSanitizer(DataFlow::Node node) {
    exists(MethodCall mc |
      mc.getTarget().hasName("Escape") and
      mc
          .getTarget()
          .getDeclaringType()
          .getABaseType*()
          .hasQualifiedName("System.Security.SecurityElement")
    |
      mc = node.asExpr()
    )
  }
}

from TaintTrackingConfiguration c, DataFlow::Node source, DataFlow::Node sink
where c.hasFlow(source, sink)
select sink, "$@ flows to here and is inserted as XML.", source, "User-provided value"

The APIs provided by the .NET libraries for XML manipulation allow the insertion of "raw" text at a specified point in an XML document. If user input is passed to this API, it could allow a malicious user to add extra content that could corrupt or supersede existing content, or enable unintended additional functionality.

Recommendation

Avoid using the WriteRaw method on System.Xml.XmlWriter with user input. If possible, use the high-level APIs to write new XML elements to a document, as these automatically escape user content. If that is not possible, then user input should be escaped before being included in a string that will be used with the WriteRaw API.

Example

In this example, user input is provided describing the name of an employee to add to an XML document representing a set of names. The WriteRaw API is used to write the new employee record to the XML file.

using System;
using System.Security;
using System.Web;
using System.Xml;

public class XMLInjectionHandler : IHttpHandler {
  public void ProcessRequest(HttpContext ctx) {
    string employeeName = ctx.Request.QueryString["employeeName"];

    using (XmlWriter writer = XmlWriter.Create("employees.xml"))
    {
        writer.WriteStartDocument();

        // BAD: Insert user input directly into XML
        writer.WriteRaw("<employee><name>" + employeeName + "</name></employee>");

        writer.WriteEndElement();
        writer.WriteEndDocument();
    }
  }
}

However, if a malicious user were to provide the content Bobby Pages</name></employee><employee><name>Hacker1, they would be able to add an extra entry into the XML file.

The corrected version demonstrates two ways to avoid this issue. The first is to escape user input before passing it to the WriteRaw API, which prevents a malicious user from closing or opening XML tags. The second approach uses the high level XML API to add XML elements, which ensures the content is appropriately escaped.

using System;
using System.Security;
using System.Web;
using System.Xml;

public class XMLInjectionHandler : IHttpHandler {
  public void ProcessRequest(HttpContext ctx) {
    string employeeName = ctx.Request.QueryString["employeeName"];

    using (XmlWriter writer = XmlWriter.Create("employees.xml"))
    {
        writer.WriteStartDocument();

        // GOOD: Escape user input before inserting into string
        writer.WriteRaw("<employee><name>" + SecurityElement.Escape(employeeName) + "</name></employee>");

        // GOOD: Use standard API, which automatically encodes values
        writer.WriteStartElement("Employee");
        writer.WriteElementString("Name", employeeName);
        writer.WriteEndElement();

        writer.WriteEndElement();
        writer.WriteEndDocument();
    }
  }

References

  • No labels