43

We had a security audit on our code, and it mentioned that our code is vulnerable to XML EXternal Entity (XXE) attacks.

Explanation

XML External Entities attacks benefit from an XML feature to build documents dynamically at the time of processing. An XML entity allows inclusion of data dynamically from a given resource. External entities allow an XML document to include data from an external URI. Unless configured to do otherwise, external entities force the XML parser to access the resource specified by the URI, e.g., a file on the local machine or on a remote system. This behavior exposes the application to XML External Entity (XXE) attacks, which can be used to perform denial of service of the local system, gain unauthorized access to files on the local machine, scan remote machines, and perform denial of service of remote systems.

The following XML document shows an example of an XXE attack.

<?xml version="1.0" encoding="ISO-8859-1"?>
<!DOCTYPE foo [
<!ELEMENT foo ANY >
<!ENTITY xxe SYSTEM "file:///dev/random" >]><foo>&xxe;</foo>

This example could crash the server (on a UNIX system), if the XML parser attempts to substitute the entity with the contents of the /dev/random file.

Recommendation

The XML unmarshaller should be configured securely so that it does not allow external entities as part of an incoming XML document.

To avoid XXE injection do not use unmarshal methods that process an XML source directly as java.io.File, java.io.Reader or java.io.InputStream. Parse the document with a securely configured parser and use an unmarshal method that takes the secure parser as the XML source as shown in the following example:

DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance();
dbf.setExpandEntityReferences(false);
DocumentBuilder db = dbf.newDocumentBuilder();
Document document = db.parse(<XML Source>);
Model model = (Model) u.unmarshal(document);

The code below is where the audit found the XXE attack:

Transformer transformer = TransformerFactory.newInstance().newTransformer();
transformer.setOutputProperty(OutputKeys.INDENT, "yes");
System.out.println("outputing to : " + outputLocation);
File outputFile = new File(outputLocation);
StreamResult result = new StreamResult(outputFile);
DOMSource source = new DOMSource(doc);
transformer.transform(source, result);

How can I implement the above recommendation in my code? Where am I missing things?

SOLO
  • 868
  • 9
  • 19
SANNO
  • 439
  • 2
  • 5
  • 7
  • 2
    OWASP has more recommendations, with good instructions, not only for Java: https://www.owasp.org/index.php/XML_External_Entity_(XXE)_Prevention_Cheat_Sheet – beat Oct 04 '18 at 15:41
  • Possible duplicate of [How to Prevent XML External Entity Injection on TransformerFactory](https://stackoverflow.com/questions/32178558/how-to-prevent-xml-external-entity-injection-on-transformerfactory) – beat Oct 05 '18 at 09:13
  • https://cheatsheetseries.owasp.org/cheatsheets/XML_External_Entity_Prevention_Cheat_Sheet.html#jaxp-documentbuilderfactory-saxparserfactory-and-dom4j – firstpostcommenter Oct 24 '22 at 11:25

4 Answers4

40

You can use the same approach with DocumentBuilderFactory:

DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
...

To make everyone use this automatically, you need to create your own implementation (by extending the one which you're currenly using; use your debugger to find out). Set the feature in the constructor.

Then you can pass the new factory to use in the System property javax.xml.parsers.DocumentBuilderFactory to the Java VM and everyone will use it.

Aaron Digulla
  • 321,842
  • 108
  • 597
  • 820
  • What's the difference between "factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);" and "factory.setExpandEntityReferences(false);"? – Ben Nov 18 '22 at 23:20
  • 1
    @Ben The latter just prevents the parser from from expanding entities. Depending on the parser, ` ` might be passed verbatim. The feature will disable all dangerous features. If there are new ones found and you update the dependency, your code will still be safe without any change. – Aaron Digulla Dec 12 '22 at 10:17
20

Note that using FEATURE_SECURE_PROCESSING alone seems not to be secure enough (from blackhat-pdf):

... despite Oracle’s recommendation XML parsers do not actually restrict external connections when FEATURE _SECURE_PROCESSING is enabled.

OWASP recommends ACCESS_EXTERNAL_DTD and ACCESS_EXTERNAL_STYLESHEET.

Together this would make:

TransformerFactory trfactory = TransformerFactory.newInstance();
trfactory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
trfactory.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, "");
trfactory.setAttribute(XMLConstants.ACCESS_EXTERNAL_STYLESHEET, "");

Actually this question is a duplicate of: How to Prevent XML External Entity Injection on TransformerFactory

beat
  • 1,857
  • 1
  • 22
  • 36
3

Because of large number of xml parsing engines in the market, the way to prevent XXE attacks differ from engine to engine. please refer to you engine documentation. The funda here is to prevent the external DOCTYPE declaration. If the external DOCTYPE declaration is needed then disabling external general entities and external parameter entities will prevent XXE attacks on your code. Below is the sample code to prevent XXE when using a SAX parser.

public class MyDocumentBuilderFactory {

 public static DocumentBuilderFactory newDocumentBuilderFactory() {

    DocumentBuilderFactory documentBuilderFactory = DocumentBuilderFactory.newInstance();

    try {
        documentBuilderFactory.setFeature("http://apache.org/xml/features/disallow-doctype-decl",true);
        
        documentBuilderFactory.setFeature("http://xml.org/sax/features/external-general-entities",false);
        
        documentBuilderFactory.setFeature("http://xml.org/sax/features/external-parameter-entities",false)


    } catch(ParserConfigurationException exp){
        exp.printStackTrace();
    }

    return documentBuilderFactory;
 }
}
Karan Khanna
  • 1,947
  • 3
  • 21
  • 49
Keerthikanth Chowdary
  • 728
  • 1
  • 10
  • 17
2

You'll want to turn on the secure processing feature on the TransformerFactory. It will limit certain possibly malicious things from happening (DOS attacks etc.)

TransformerFactory tf = TransformerFactory.newInstance();
tf.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
Transformer transformer = tf.newTransformer();
Kayaman
  • 72,141
  • 5
  • 83
  • 121
  • Thanks for responding, Yes, but i need to implement DocumentBuilderFactory in existing logic.... btw i used your given suggestion but getting error "The method setFeature(String, boolean) is undefined for the type Transformer". – SANNO Nov 17 '16 at 11:11
  • Eh, sorry, you need to set it in the TransformerFactory. – Kayaman Nov 17 '16 at 11:17
  • Thank you Kayaman ! its working fine but can you help to replace TransformerFactory concept by DocumentBuilderFactory concept. Really it will be appreciable. – SANNO Nov 18 '16 at 05:37
  • Can anyone help? Anyone help is appreciated ! – SANNO Nov 21 '16 at 06:04