Merge remote-tracking branch 'origin/master'
This commit is contained in:
commit
825e7cd128
|
@ -11,17 +11,49 @@
|
|||
<module name="TreeWalker">
|
||||
<!--
|
||||
<module name="TodoComment">-->
|
||||
<!-- The (?i) below means Case Insensitive -->
|
||||
<!-- The (?i) below means Case Insensitive -->
|
||||
<!--<property name="format" value="(?i)FIXME"/>
|
||||
-->
|
||||
<module name="RegexpSinglelineJava">
|
||||
<property name="format" value="org\.jetbrains\.annotations\.NotNull"/>
|
||||
</module>
|
||||
<module name="RegexpSinglelineJava">
|
||||
<property name="format" value="org\.jetbrains\.annotations\.Nullable"/>
|
||||
</module>
|
||||
<module name="RegexpSinglelineJava">
|
||||
<property name="format" value="org\.jetbrains\.annotations\.\*"/>
|
||||
</module>
|
||||
</module>
|
||||
<module name="RegexpSinglelineJava">
|
||||
<property name="format" value="org\.jetbrains\.annotations\.NotNull"/>
|
||||
</module>
|
||||
<module name="RegexpSinglelineJava">
|
||||
<property name="format" value="org\.jetbrains\.annotations\.Nullable"/>
|
||||
</module>
|
||||
<module name="RegexpSinglelineJava">
|
||||
<property name="format" value="org\.jetbrains\.annotations\.\*"/>
|
||||
</module>
|
||||
</module>
|
||||
<module name="RegexpMultiline">
|
||||
<property name="id" value="transformerFactoryNewInstance"/>
|
||||
<property name="matchAcrossLines" value="true"/>
|
||||
<property name="format" value="TransformerFactory\.newInstance\("/>
|
||||
<property name="message"
|
||||
value="Usage of TransformerFactory.newInstance() is only allowed in XMLUtil.newXXEProtectedTransformerFactory(). If the location of this call in XMLUtil has changed, please modify the expected line number in org.hl7.fhir.utilities/checkstyle_suppressions.xml"
|
||||
/>
|
||||
</module>
|
||||
<module name="RegexpMultiline">
|
||||
<property name="id" value="documentBuilderFactoryNewInstance"/>
|
||||
<property name="matchAcrossLines" value="true"/>
|
||||
<property name="format" value="DocumentBuilderFactory\.newInstance\("/>
|
||||
<property name="message"
|
||||
value="Usage of DocumentBuilderFactory.newInstance() is only allowed in XMLUtil.newXXEProtectedDocumentBuilderFactory(). If the location of this call in XMLUtil has changed, please modify the expected line number in org.hl7.fhir.utilities/checkstyle_suppressions.xml"
|
||||
/>
|
||||
</module>
|
||||
<module name="RegexpMultiline">
|
||||
<property name="id" value="saxParserFactoryNewInstance"/>
|
||||
<property name="matchAcrossLines" value="true"/>
|
||||
<property name="format" value="SAXParserFactory\.newInstance\("/>
|
||||
<property name="message"
|
||||
value="Usage of SAXParserFactory.newInstance() is only allowed in XMLUtil.newXXEProtectedSaxParserFactory(). If the location of this call in XMLUtil has changed, please modify the expected line number in org.hl7.fhir.utilities/checkstyle_suppressions.xml"
|
||||
/>
|
||||
</module>
|
||||
<module name="RegexpMultiline">
|
||||
<property name="id" value="getXMLReader"/>
|
||||
<property name="matchAcrossLines" value="true"/>
|
||||
<property name="format" value="\.getXMLReader\("/>
|
||||
<property name="message"
|
||||
value="Usage of SAXParserFactory.getXMLReader() is only allowed in XMLUtil.getXXEProtectedXMLReader(...). If the location of this call in XMLUtil has changed, please modify the expected line number in org.hl7.fhir.utilities/checkstyle_suppressions.xml"
|
||||
/>
|
||||
</module>
|
||||
</module>
|
|
@ -4,5 +4,5 @@
|
|||
"https://checkstyle.org/dtds/suppressions_1_2.dtd">
|
||||
|
||||
<suppressions>
|
||||
|
||||
|
||||
</suppressions>
|
||||
|
|
|
@ -4,5 +4,8 @@
|
|||
"https://checkstyle.org/dtds/suppressions_1_2.dtd">
|
||||
|
||||
<suppressions>
|
||||
|
||||
<suppress id="transformerFactoryNewInstance" files="/src/main/java/org/hl7/fhir/utilities/xml/XMLUtil.java" lines="516"/>
|
||||
<suppress id="documentBuilderFactoryNewInstance" files="/src/main/java/org/hl7/fhir/utilities/xml/XMLUtil.java" lines="532"/>
|
||||
<suppress id="saxParserFactoryNewInstance" files="/src/main/java/org/hl7/fhir/utilities/xml/XMLUtil.java" lines="551"/>
|
||||
<suppress id="getXMLReader" files="/src/main/java/org/hl7/fhir/utilities/xml/XMLUtil.java" lines="569"/>
|
||||
</suppressions>
|
||||
|
|
|
@ -504,6 +504,14 @@ public class XMLUtil {
|
|||
return e == null ? null : e.getAttribute(aname);
|
||||
}
|
||||
|
||||
/**
|
||||
* This method is used to create a new TransformerFactory instance with external processing features configured
|
||||
* securely.
|
||||
* <p/>
|
||||
* <b>IMPORTANT</b> This method should be the only place where TransformerFactory is instantiated in this project.
|
||||
*
|
||||
* @return A TransformerFactory instance external processing features configured securely.
|
||||
*/
|
||||
public static TransformerFactory newXXEProtectedTransformerFactory() {
|
||||
final TransformerFactory transformerFactory = TransformerFactory.newInstance();
|
||||
transformerFactory.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, "");
|
||||
|
@ -511,6 +519,15 @@ public class XMLUtil {
|
|||
return transformerFactory;
|
||||
}
|
||||
|
||||
/**
|
||||
* This method is used to create a new DocumentBuilderFactory instance with external processing features configured
|
||||
* securely.
|
||||
* <p/>
|
||||
* <b>IMPORTANT</b> This method should be the only place where DocumentBuilderFactory is instantiated in this project.
|
||||
*
|
||||
* @return A DocumentBuilderFactory instance external processing features configured securely.
|
||||
* @throws ParserConfigurationException If a DocumentBuilder cannot be configured with the requested features.
|
||||
*/
|
||||
public static DocumentBuilderFactory newXXEProtectedDocumentBuilderFactory() throws ParserConfigurationException {
|
||||
final DocumentBuilderFactory documentBuilderFactory = DocumentBuilderFactory.newInstance();
|
||||
documentBuilderFactory.setFeature(APACHE_XML_FEATURES_DISALLOW_DOCTYPE_DECL, true);
|
||||
|
@ -518,13 +535,35 @@ public class XMLUtil {
|
|||
return documentBuilderFactory;
|
||||
}
|
||||
|
||||
/**
|
||||
* This method is used to create a new SAXParserFactory instance with external processing features configured
|
||||
* securely.
|
||||
* <p/>
|
||||
* <b>IMPORTANT</b> This method should be the only place where SAXParserFactory is instantiated in this project.
|
||||
*
|
||||
* @return A SAXParserFactory instance external processing features configured securely.
|
||||
* @throws SAXNotSupportedException When the underlying XMLReader recognizes the property name but doesn't support
|
||||
* the property.
|
||||
* @throws SAXNotRecognizedException When the underlying XMLReader does not recognize the property name.
|
||||
* @throws ParserConfigurationException If a SAXParser cannot be configured with the requested features.
|
||||
*/
|
||||
public static SAXParserFactory newXXEProtectedSaxParserFactory() throws SAXNotSupportedException, SAXNotRecognizedException, ParserConfigurationException {
|
||||
final SAXParserFactory spf = SAXParserFactory.newInstance();
|
||||
spf.setFeature(SAX_FEATURES_EXTERNAL_GENERAL_ENTITIES, false);
|
||||
spf.setFeature(APACHE_XML_FEATURES_DISALLOW_DOCTYPE_DECL, true);
|
||||
|
||||
return spf;
|
||||
}
|
||||
|
||||
/**
|
||||
* This method is used to create a new XMLReader instance from a passed SAXParserFactory with external processing
|
||||
* features configured securely.
|
||||
*
|
||||
* @param spf The SAXParserFactory to create the XMLReader from.
|
||||
* @return A XMLReader instance external processing features configured securely.
|
||||
* @throws ParserConfigurationException If a SAXParser cannot be configured with the requested features.
|
||||
* @throws SAXException If any SAX exceptions occur during the creation of the XMLReader.
|
||||
*/
|
||||
public static XMLReader getXXEProtectedXMLReader(SAXParserFactory spf) throws ParserConfigurationException, SAXException {
|
||||
final SAXParser saxParser = spf.newSAXParser();
|
||||
final XMLReader xmlReader = saxParser.getXMLReader();
|
||||
|
|
|
@ -0,0 +1,87 @@
|
|||
package org.hl7.fhir.utilities.xml;
|
||||
|
||||
import net.sf.saxon.trans.XPathException;
|
||||
import org.hl7.fhir.utilities.filesystem.ManagedFileAccess;
|
||||
import org.junit.Test;
|
||||
import org.w3c.dom.Document;
|
||||
import org.xml.sax.InputSource;
|
||||
import org.xml.sax.SAXException;
|
||||
import org.xml.sax.SAXParseException;
|
||||
import org.xml.sax.XMLReader;
|
||||
|
||||
import javax.xml.parsers.DocumentBuilder;
|
||||
import javax.xml.parsers.DocumentBuilderFactory;
|
||||
import javax.xml.parsers.ParserConfigurationException;
|
||||
import javax.xml.parsers.SAXParserFactory;
|
||||
import javax.xml.transform.*;
|
||||
import javax.xml.transform.dom.DOMResult;
|
||||
import javax.xml.transform.dom.DOMSource;
|
||||
import javax.xml.transform.sax.SAXSource;
|
||||
import javax.xml.transform.stream.StreamResult;
|
||||
import javax.xml.transform.stream.StreamSource;
|
||||
|
||||
import java.io.File;
|
||||
import java.io.IOException;
|
||||
import java.io.StringWriter;
|
||||
|
||||
import static org.assertj.core.api.Assertions.assertThat;
|
||||
import static org.junit.jupiter.api.Assertions.assertThrows;
|
||||
|
||||
public class XMLUtilTests {
|
||||
@Test
|
||||
public void testDocumentBuilderThrowsExceptionForExternalEntity() throws ParserConfigurationException, IOException {
|
||||
DocumentBuilderFactory factory = XMLUtil.newXXEProtectedDocumentBuilderFactory();
|
||||
DocumentBuilder safeBuilder = factory.newDocumentBuilder();
|
||||
|
||||
File file = ManagedFileAccess.file("src/test/resources/xml/evil-resource.xml");
|
||||
|
||||
SAXParseException e = assertThrows(SAXParseException.class, ()-> safeBuilder.parse(file));
|
||||
assertThat(e.getMessage()).contains("DOCTYPE is disallowed");
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testTransformerFactoryThrowsExceptionForExternalEntity() throws ParserConfigurationException, IOException, SAXException, TransformerException {
|
||||
DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
|
||||
DocumentBuilder safeBuilder = factory.newDocumentBuilder();
|
||||
|
||||
File file = ManagedFileAccess.file("src/test/resources/xml/resource.xml");
|
||||
|
||||
Document document = safeBuilder.parse(file);
|
||||
|
||||
StringWriter sw = new StringWriter();
|
||||
TransformerFactory tf = XMLUtil.newXXEProtectedTransformerFactory();
|
||||
|
||||
File templateFile = ManagedFileAccess.file("src/test/resources/xml/evil-transform.xslt");
|
||||
Source xsltSource = new StreamSource(templateFile);
|
||||
Transformer transformer = tf.newTransformer(xsltSource);
|
||||
transformer.setOutputProperty(OutputKeys.OMIT_XML_DECLARATION, "no");
|
||||
transformer.setOutputProperty(OutputKeys.METHOD, "xml");
|
||||
transformer.setOutputProperty(OutputKeys.INDENT, "yes");
|
||||
transformer.setOutputProperty(OutputKeys.ENCODING, "UTF-8");
|
||||
|
||||
XPathException e = assertThrows(XPathException.class, () -> transformer.transform(new DOMSource(document), new StreamResult(sw)));
|
||||
assertThat(e.getMessage()).contains("URIs using protocol file are not permitted");
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testSaxParserFactoryThrowsExceptionForExternalEntity() throws ParserConfigurationException, IOException, SAXException, TransformerException {
|
||||
|
||||
SAXParserFactory spf = XMLUtil.newXXEProtectedSaxParserFactory();
|
||||
spf.setNamespaceAware(true);
|
||||
spf.setValidating(false);
|
||||
XMLReader xmlReader = spf.newSAXParser().getXMLReader();
|
||||
|
||||
File templateFile = ManagedFileAccess.file("src/test/resources/xml/evil-resource.xml");
|
||||
|
||||
SAXParseException e = assertThrows(SAXParseException.class, () -> xmlReader.parse(new StreamSource(templateFile).getSystemId()));
|
||||
assertThat(e.getMessage()).contains("DOCTYPE is disallowed");
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testXMLReaderThrowsExceptionForExternalEntity() throws ParserConfigurationException, IOException, SAXException, TransformerException {
|
||||
SAXParserFactory spf = SAXParserFactory.newInstance();
|
||||
|
||||
IllegalArgumentException e = assertThrows(IllegalArgumentException.class, () -> XMLUtil.getXXEProtectedXMLReader(spf));
|
||||
assertThat(e.getMessage()).contains("SAXParserFactory has insecure feature setting");
|
||||
}
|
||||
}
|
|
@ -0,0 +1,6 @@
|
|||
<?xml version="1.0" encoding="UTF-8"?>
|
||||
<!DOCTYPE foo [<!ENTITY example SYSTEM "jackpot.xml"> ]>
|
||||
<Patient xmlns="http://hl7.org/fhir">
|
||||
<id value="example"/>
|
||||
<text> <status value="generated"/> &example;</text>
|
||||
</Patient>
|
|
@ -0,0 +1,6 @@
|
|||
<xsl:stylesheet version="1.0"
|
||||
xmlns:xsl="http://www.w3.org/1999/XSL/Transform">
|
||||
<xsl:template match="/">
|
||||
Evil: <xsl:value-of select="document('jackpot.xml')" />
|
||||
</xsl:template>
|
||||
</xsl:stylesheet>
|
|
@ -0,0 +1 @@
|
|||
<stuff>stuff</stuff>
|
|
@ -0,0 +1,5 @@
|
|||
<?xml version="1.0" encoding="UTF-8"?>
|
||||
|
||||
<Patient xmlns="http://hl7.org/fhir">
|
||||
<id value="example"/>
|
||||
</Patient>
|
Loading…
Reference in New Issue