Skip to content

Resolve vulnerability: Improper restriction of XML external entity reference ('XXE')

Paul Dumaitre requested to merge remediate/improper-restriction-of-xml-external-entity-reference---xxe-D20240919T075605 into develop

MR created from vulnerability: Improper restriction of XML external entity reference ('XXE')

AI GENERATED PATCH

The suggested code changes were generated by GitLab Duo Vulnerability Resolution, an AI feature. Use this feature with caution. Before you run a pipeline or apply the code changes, carefully review and test them, to ensure that they solve the vulnerability.

The large language model that generated the suggested code changes was provided with the entire file that contains the vulnerable lines of code. It is not aware of any functionality outside of this context.

Please see our documentation for more information about this feature.

Description:

External XML entities are a feature of XML parsers that allow documents to contain references to other documents or data. This feature can be abused to read files, communicate with external hosts, exfiltrate data, or cause a Denial of Service (DoS).

In most XML parsers, the recommendation to protect against XXE is to disable the doctype feature. Unfortunately use of the XMLInputFactory requires that the doctypes feature be enabled. Instead the application can set the ACCESS_EXTERNAL_DTD to an empty string and disable javax.xml.stream.isSupportingExternalEntities.

Creates an XMLInputFactory stream parser, but disables accessing external DTD or entities:

// Create an XMLInputFactory
XMLInputFactory factory = XMLInputFactory.newFactory();
// Set the ACCESS_EXTERNAL_DTD property to an empty string so it won't access
// entities using protocols
// (ref:
https://docs.oracle.com/javase/8/docs/api/javax/xml/XMLConstants.html#ACCESS_EXTERNAL_DTD)
factory.setProperty(XMLConstants.ACCESS_EXTERNAL_DTD, "");
// Additionally, disable support for resolving external entities
factory.setProperty("javax.xml.stream.isSupportingExternalEntities", false);
// Continue to work with the factory/stream parser

For more information on XML security see OWASP's guide: https://cheatsheetseries.owasp.org/cheatsheets/XML_External_Entity_Prevention_Cheat_Sheet.html#java

Analysis:

The vulnerability report indicates an "Improper restriction of XML external entity reference ('XXE')" issue, which is associated with CWE-611. This is a serious security vulnerability that can lead to various attacks, including information disclosure, denial of service, and server-side request forgery.

The vulnerable code is in the parseXml method of the Comments class. Specifically, the XMLInputFactory is configured to support external entities and DTDs, which is the root cause of the XXE vulnerability. The createXMLStreamReader method is then used with these unsafe settings to parse the XML input.

This configuration allows the XML parser to process external entities, which can be exploited by an attacker to read arbitrary files on the server, make network connections to internal systems, or perform denial of service attacks.

The vulnerability is not a false positive, as the code explicitly enables features that are known to be unsafe when processing untrusted XML input.

Summary:

  1. The reported vulnerability is an XML External Entity (XXE) injection, identified as CWE-611. This vulnerability allows an attacker to potentially read arbitrary files on the server, make network connections to internal systems, or perform denial of service attacks by exploiting the XML parser's ability to process external entities.

  2. The fix addresses the security concern by disabling the support for external entities and DTDs in the XML parser configuration. Specifically:

    • XMLInputFactory.IS_SUPPORTING_EXTERNAL_ENTITIES is set to false
    • XMLInputFactory.SUPPORT_DTD is set to false

    These changes prevent the XML parser from processing external entities, effectively mitigating the XXE vulnerability. The fix maintains the original functionality of parsing XML input but does so in a secure manner.

  3. It's important to note that this fix assumes that external entities and DTDs are not required for the legitimate functionality of the application. If these features are needed, a more complex solution involving whitelisting or sanitization of input might be necessary. However, in most cases, disabling these features is the safest approach.

The fixed code ensures that the parseXml method can still process XML input for comments, but does so without the risk of XXE attacks. This change significantly improves the security posture of the application without altering its core functionality.

Identifiers:

  • CWE-611
  • find_sec_bugs.XXE_XMLSTREAMREADER-1
  • Find Security Bugs-XXE_XMLSTREAMREADER
  • A1:2017 - Injection
  • A03:2021 - Injection

Merge request reports