diff --git a/weixin4j-base/src/main/java/com/foxinmy/weixin4j/xml/XmlStream.java b/weixin4j-base/src/main/java/com/foxinmy/weixin4j/xml/XmlStream.java index 79b479bd..a002cc2a 100644 --- a/weixin4j-base/src/main/java/com/foxinmy/weixin4j/xml/XmlStream.java +++ b/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); - spf.setFeature("http://xml.org/sax/features/external-parameter-entities", false); - spf.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false); + // This is the PRIMARY defense. If DTDs (doctypes) are disallowed, + // almost all XML entity attacks are prevented + // 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)); - XmlRootElement rootElement = clazz.getAnnotation(XmlRootElement.class); + XMLReader reader = spf.newSAXParser().getXMLReader(); + 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())) { - JAXBElement jaxbElement = unmarshaller.unmarshal(source, clazz); + || rootElement.name().equals( + XmlRootElement.class.getMethod("name") + .getDefaultValue().toString())) { + JAXBElement 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 fromXML(String content, Class 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 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 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 entry : json.entrySet()) { @@ -191,7 +243,8 @@ public final class XmlStream { Map map = new HashMap(); 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()); - XmlRootElement rootElement = clazz.getAnnotation(XmlRootElement.class); + marshaller.setProperty(Marshaller.JAXB_ENCODING, + Consts.UTF_8.name()); + XmlRootElement rootElement = clazz + .getAnnotation(XmlRootElement.class); if (rootElement == null - || rootElement.name().equals(XmlRootElement.class.getMethod("name").getDefaultValue().toString())) { - marshaller.marshal(new JAXBElement(new QName(ROOT_ELEMENT_XML), clazz, t), os); + || rootElement.name().equals( + XmlRootElement.class.getMethod("name") + .getDefaultValue().toString())) { + marshaller.marshal(new JAXBElement(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;