Added XXE protection.

#55
mangstadt · Jun 4, 2016 · e7822b7 · e7822b7
1 parent 8b26b8c
commit e7822b7
Show file tree

Hide file tree

Showing 2 changed files with 64 additions and 2 deletions.
diff --git a/src/main/java/ezvcard/io/xml/XCardReader.java b/src/main/java/ezvcard/io/xml/XCardReader.java
@@ -101,6 +101,7 @@
  *   if (reader != null) reader.close();
  * }
  * </pre>
+ * 
  * @author Michael Angstadt
  * @see <a href="http://tools.ietf.org/html/rfc6351">RFC 6351</a>
  */
@@ -201,7 +202,10 @@ public ReadThread() {
 
 			//create the transformer
 			try {
-				transformer = TransformerFactory.newInstance().newTransformer();
+				TransformerFactory factory = TransformerFactory.newInstance();
+				XmlUtils.applyXXEProtection(factory);
+
+				transformer = factory.newTransformer();
 			} catch (TransformerConfigurationException e) {
 				//shouldn't be thrown because it's a simple configuration
 				throw new RuntimeException(e);

diff --git a/src/main/java/ezvcard/util/XmlUtils.java b/src/main/java/ezvcard/util/XmlUtils.java
@@ -151,6 +151,7 @@ private static Document toDocument(InputSource in) throws SAXException, IOExcept
 		DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
 		factory.setNamespaceAware(true);
 		factory.setIgnoringComments(true);
+		applyXXEProtection(factory);
 
 		DocumentBuilder builder;
 		try {
@@ -163,6 +164,63 @@ private static Document toDocument(InputSource in) throws SAXException, IOExcept
 		return builder.parse(in);
 	}
 
+	/**
+	 * Configures a {@link DocumentBuilderFactory} to protect it against XML
+	 * External Entity attacks.
+	 * @param factory the factory
+	 * @see <a href=
+	 * "https://www.owasp.org/index.php/XML_External_Entity_%28XXE%29_Prevention_Cheat_Sheet#Java">
+	 * XXE Cheat Sheet</a>
+	 */
+	public static void applyXXEProtection(DocumentBuilderFactory factory) {
+		Map<String, Boolean> features = new HashMap<String, Boolean>();
+		features.put("http://apache.org/xml/features/disallow-doctype-decl", true);
+		features.put("http://xml.org/sax/features/external-general-entities", false);
+		features.put("http://xml.org/sax/features/external-parameter-entities", false);
+		features.put("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
+
+		for (Map.Entry<String, Boolean> entry : features.entrySet()) {
+			String feature = entry.getKey();
+			Boolean value = entry.getValue();
+			try {
+				factory.setFeature(feature, value);
+			} catch (ParserConfigurationException e) {
+				//feature is not supported by the local XML engine, skip it
+			}
+		}
+
+		factory.setXIncludeAware(false);
+		factory.setExpandEntityReferences(false);
+	}
+
+	/**
+	 * Configures a {@link TransformerFactory} to protect it against XML
+	 * External Entity attacks.
+	 * @param factory the factory
+	 * @see <a href=
+	 * "https://www.owasp.org/index.php/XML_External_Entity_%28XXE%29_Prevention_Cheat_Sheet#Java">
+	 * XXE Cheat Sheet</a>
+	 */
+	public static void applyXXEProtection(TransformerFactory factory) {
+		//@formatter:off
+		String[] attributes = {
+			//XMLConstants.ACCESS_EXTERNAL_DTD (Java 7 only)
+			"http://javax.xml.XMLConstants/property/accessExternalDTD",
+
+			//XMLConstants.ACCESS_EXTERNAL_STYLESHEET (Java 7 only)
+			"http://javax.xml.XMLConstants/property/accessExternalStylesheet"
+		};
+		//@formatter:on
+
+		for (String attribute : attributes) {
+			try {
+				factory.setAttribute(attribute, "");
+			} catch (IllegalArgumentException e) {
+				//attribute is not supported by the local XML engine, skip it
+			}
+		}
+	}
+
 	/**
 	 * Converts an XML node to a string.
 	 * @param node the XML node
@@ -295,4 +353,4 @@ public static boolean hasQName(Node node, QName qname) {
 	private XmlUtils() {
 		//hide
 	}
-}
+}