fixed XXE bug

weixin4j-base/src/main/java/com/foxinmy/weixin4j/xml/XmlStream.java

@@ -30,6 +30,7 @@ import javax.xml.transform.Source;
 import javax.xml.transform.sax.SAXSource;
 
 import org.xml.sax.InputSource;
+import org.xml.sax.XMLReader;
 
 import com.alibaba.fastjson.JSONObject;
 import com.foxinmy.weixin4j.util.Consts;
@@ -51,9 +52,38 @@ public final class XmlStream {
 	private final static SAXParserFactory spf = SAXParserFactory.newInstance();
 	static {
 		try {
-			spf.setFeature("http://xml.org/sax/features/external-general-entities", false);
+			// This is the PRIMARY defense. If DTDs (doctypes) are disallowed,
-			spf.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
+			// almost all XML entity attacks are prevented
-			spf.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
+			// Xerces 2 only -
+			// http://xerces.apache.org/xerces2-j/features.html#disallow-doctype-decl
+			spf.setFeature(
+					"http://apache.org/xml/features/disallow-doctype-decl",
+					true);
+			// If you can't completely disable DTDs, then at least do the
+			// following:
+			// Xerces 1 -
+			// http://xerces.apache.org/xerces-j/features.html#external-general-entities
+			// Xerces 2 -
+			// http://xerces.apache.org/xerces2-j/features.html#external-general-entities
+			// JDK7+ - http://xml.org/sax/features/external-general-entities
+			spf.setFeature(
+					"http://xml.org/sax/features/external-general-entities",
+					false);
+			// Xerces 1 -
+			// http://xerces.apache.org/xerces-j/features.html#external-parameter-entities
+			// Xerces 2 -
+			// http://xerces.apache.org/xerces2-j/features.html#external-parameter-entities
+			// JDK7+ - http://xml.org/sax/features/external-parameter-entities
+			spf.setFeature(
+					"http://xml.org/sax/features/external-parameter-entities",
+					false);
+			// Disable external DTDs as well
+			spf.setFeature(
+					"http://apache.org/xml/features/nonvalidating/load-external-dtd",
+					false);
+			// and these as well, per Timothy Morgan's 2014 paper:
+			// "XML Schema, DTD, and Entity Attacks"
+			spf.setXIncludeAware(false);
 		} catch (Exception e) {
 			;
 		}
@@ -73,17 +103,36 @@ public final class XmlStream {
 		JAXBContext jaxbContext = getJaxbContext(clazz);
 		try {
 			Unmarshaller unmarshaller = jaxbContext.createUnmarshaller();
-			Source source = new SAXSource(spf.newSAXParser().getXMLReader(), new InputSource(content));
+			XMLReader reader = spf.newSAXParser().getXMLReader();
-			XmlRootElement rootElement = clazz.getAnnotation(XmlRootElement.class);
+			reader.setFeature(
+					"http://apache.org/xml/features/disallow-doctype-decl",
+					true);
+			reader.setFeature(
+					"http://apache.org/xml/features/nonvalidating/load-external-dtd",
+					false); // This may not be strictly required as DTDs
+							// shouldn't be allowed at all, per previous line.
+			reader.setFeature(
+					"http://xml.org/sax/features/external-general-entities",
+					false);
+			reader.setFeature(
+					"http://xml.org/sax/features/external-parameter-entities",
+					false);
+			Source source = new SAXSource(reader, new InputSource(content));
+			XmlRootElement rootElement = clazz
+					.getAnnotation(XmlRootElement.class);
 			if (rootElement == null
-					|| rootElement.name().equals(XmlRootElement.class.getMethod("name").getDefaultValue().toString())) {
+					|| rootElement.name().equals(
-				JAXBElement<T> jaxbElement = unmarshaller.unmarshal(source, clazz);
+							XmlRootElement.class.getMethod("name")
+									.getDefaultValue().toString())) {
+				JAXBElement<T> jaxbElement = unmarshaller.unmarshal(source,
+						clazz);
 				return jaxbElement.getValue();
 			} else {
 				return (T) unmarshaller.unmarshal(source);
 			}
 		} catch (Exception ex) {
-			throw new RuntimeException("Could not unmarshaller class [" + clazz + "]", ex);
+			throw new RuntimeException("Could not unmarshaller class [" + clazz
+					+ "]", ex);
 		} finally {
 			if (content != null) {
 				try {
@@ -105,7 +154,8 @@ public final class XmlStream {
 	 * @return
 	 */
 	public static <T> T fromXML(String content, Class<T> clazz) {
-		return fromXML(new ByteArrayInputStream(content.getBytes(Consts.UTF_8)), clazz);
+		return fromXML(
+				new ByteArrayInputStream(content.getBytes(Consts.UTF_8)), clazz);
 	}
 
 	/**
@@ -118,7 +168,8 @@ public final class XmlStream {
 	public static String map2xml(Map<String, String> map) {
 		StringWriter sw = new StringWriter();
 		try {
-			XMLStreamWriter xw = XMLOutputFactory.newInstance().createXMLStreamWriter(sw);
+			XMLStreamWriter xw = XMLOutputFactory.newInstance()
+					.createXMLStreamWriter(sw);
 			xw.writeStartDocument(Consts.UTF_8.name(), XML_VERSION);
 			xw.writeStartElement(ROOT_ELEMENT_XML);
 			for (Entry<String, String> entry : map.entrySet()) {
@@ -154,7 +205,8 @@ public final class XmlStream {
 	public static String map2xml(JSONObject json) {
 		StringWriter sw = new StringWriter();
 		try {
-			XMLStreamWriter xw = XMLOutputFactory.newInstance().createXMLStreamWriter(sw);
+			XMLStreamWriter xw = XMLOutputFactory.newInstance()
+					.createXMLStreamWriter(sw);
 			xw.writeStartDocument(Consts.UTF_8.name(), XML_VERSION);
 			xw.writeStartElement(ROOT_ELEMENT_XML);
 			for (Entry<String, Object> entry : json.entrySet()) {
@@ -191,7 +243,8 @@ public final class XmlStream {
 		Map<String, String> map = new HashMap<String, String>();
 		StringReader sr = new StringReader(content);
 		try {
-			XMLStreamReader xr = XMLInputFactory.newInstance().createXMLStreamReader(sr);
+			XMLStreamReader xr = XMLInputFactory.newInstance()
+					.createXMLStreamReader(sr);
 			while (true) {
 				int event = xr.next();
 				if (event == XMLStreamConstants.END_DOCUMENT) {
@@ -247,16 +300,22 @@ public final class XmlStream {
 		JAXBContext jaxbContext = getJaxbContext(clazz);
 		try {
 			Marshaller marshaller = jaxbContext.createMarshaller();
-			marshaller.setProperty(Marshaller.JAXB_ENCODING, Consts.UTF_8.name());
+			marshaller.setProperty(Marshaller.JAXB_ENCODING,
-			XmlRootElement rootElement = clazz.getAnnotation(XmlRootElement.class);
+					Consts.UTF_8.name());
+			XmlRootElement rootElement = clazz
+					.getAnnotation(XmlRootElement.class);
 			if (rootElement == null
-					|| rootElement.name().equals(XmlRootElement.class.getMethod("name").getDefaultValue().toString())) {
+					|| rootElement.name().equals(
-				marshaller.marshal(new JAXBElement<T>(new QName(ROOT_ELEMENT_XML), clazz, t), os);
+							XmlRootElement.class.getMethod("name")
+									.getDefaultValue().toString())) {
+				marshaller.marshal(new JAXBElement<T>(new QName(
+						ROOT_ELEMENT_XML), clazz, t), os);
 			} else {
 				marshaller.marshal(t, os);
 			}
 		} catch (Exception ex) {
-			throw new RuntimeException("Could not marshal class [" + clazz + "] ", ex);
+			throw new RuntimeException("Could not marshal class [" + clazz
+					+ "] ", ex);
 		} finally {
 			if (os != null) {
 				try {
@@ -275,7 +334,9 @@ public final class XmlStream {
 				jaxbContext = JAXBContext.newInstance(clazz);
 				jaxbContexts.putIfAbsent(clazz, jaxbContext);
 			} catch (JAXBException ex) {
-				throw new RuntimeException("Could not instantiate JAXBContext for class [" + clazz + "] ", ex);
+				throw new RuntimeException(
+						"Could not instantiate JAXBContext for class [" + clazz
+								+ "] ", ex);
 			}
 		}
 		return jaxbContext;