Disable DTD when parsing untrusted XML input

Issue: SPR-13136

spring-oxm/src/main/java/org/springframework/oxm/jaxb/Jaxb2Marshaller.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2014 the original author or authors.
+ * Copyright 2002-2015 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -176,6 +176,8 @@ public class Jaxb2Marshaller implements MimeMarshaller, MimeUnmarshaller, Generi
 
 	private Schema schema;
 
+	private boolean supportDtd = false;
+
 	private boolean processExternalEntities = false;
 
 
@@ -390,6 +392,21 @@ public class Jaxb2Marshaller implements MimeMarshaller, MimeUnmarshaller, Generi
 		this.mappedClass = mappedClass;
 	}
 
+	/**
+	 * Indicates whether DTD parsing should be supported.
+	 * <p>Default is {@code false} meaning that DTD is disabled.
+	 */
+	public void setSupportDtd(boolean supportDtd) {
+		this.supportDtd = supportDtd;
+	}
+
+	/**
+	 * Whether DTD parsing is supported.
+	 */
+	public boolean isSupportDtd() {
+		return this.supportDtd;
+	}
+
 	/**
 	 * Indicates whether external XML entities are processed when unmarshalling.
 	 * <p>Default is {@code false}, meaning that external entities are not resolved.
@@ -397,9 +414,14 @@ public class Jaxb2Marshaller implements MimeMarshaller, MimeUnmarshaller, Generi
 	 * {@code Source} passed to {@link #unmarshal(Source)} is a {@link SAXSource} or
 	 * {@link StreamSource}. It has no effect for {@link DOMSource} or {@link StAXSource}
 	 * instances.
+	 * <p><strong>Note:</strong> setting this option to {@code true} also
+	 * automatically sets {@link #setSupportDtd} to {@code true}.
 	 */
 	public void setProcessExternalEntities(boolean processExternalEntities) {
 		this.processExternalEntities = processExternalEntities;
+		if (processExternalEntities) {
+			setSupportDtd(true);
+		}
 	}
 
 	/**
@@ -746,6 +768,14 @@ public class Jaxb2Marshaller implements MimeMarshaller, MimeUnmarshaller, Generi
 				return unmarshaller.unmarshal(source);
 			}
 		}
+		catch (NullPointerException ex) {
+			if (!isSupportDtd()) {
+				throw new UnmarshallingFailureException("NPE while unmarshalling. " +
+						"This can happen on JDK 1.6 due to the presence of DTD " +
+						"declarations, which are disabled.", ex);
+			}
+			throw ex;
+		}
 		catch (JAXBException ex) {
 			throw convertJaxbException(ex);
 		}
@@ -801,6 +831,7 @@ public class Jaxb2Marshaller implements MimeMarshaller, MimeUnmarshaller, Generi
 			if (xmlReader == null) {
 				xmlReader = XMLReaderFactory.createXMLReader();
 			}
+			xmlReader.setFeature("http://apache.org/xml/features/disallow-doctype-decl", !isSupportDtd());
 			String name = "http://xml.org/sax/features/external-general-entities";
 			xmlReader.setFeature(name, isProcessExternalEntities());
 			if (!isProcessExternalEntities()) {

spring-oxm/src/main/java/org/springframework/oxm/support/AbstractMarshaller.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2014 the original author or authors.
+ * Copyright 2002-2015 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -71,6 +71,8 @@ public abstract class AbstractMarshaller implements Marshaller, Unmarshaller {
 	/** Logger available to subclasses */
 	protected final Log logger = LogFactory.getLog(getClass());
 
+	private boolean supportDtd = false;
+
 	private boolean processExternalEntities = false;
 
 	private DocumentBuilderFactory documentBuilderFactory;
@@ -78,6 +80,21 @@ public abstract class AbstractMarshaller implements Marshaller, Unmarshaller {
 	private final Object documentBuilderFactoryMonitor = new Object();
 
 
+	/**
+	 * Indicates whether DTD parsing should be supported.
+	 * <p>Default is {@code false} meaning that DTD is disabled.
+	 */
+	public void setSupportDtd(boolean supportDtd) {
+		this.supportDtd = supportDtd;
+	}
+
+	/**
+	 * Whether DTD parsing is supported.
+	 */
+	public boolean isSupportDtd() {
+		return this.supportDtd;
+	}
+
 	/**
 	 * Indicates whether external XML entities are processed when unmarshalling.
 	 * <p>Default is {@code false}, meaning that external entities are not resolved.
@@ -85,9 +102,14 @@ public abstract class AbstractMarshaller implements Marshaller, Unmarshaller {
 	 * {@code Source} passed to {@link #unmarshal(Source)} is a {@link SAXSource} or
 	 * {@link StreamSource}. It has no effect for {@link DOMSource} or {@link StAXSource}
 	 * instances.
+	 * <p><strong>Note:</strong> setting this option to {@code true} also
+	 * automatically sets {@link #setSupportDtd} to {@code true}.
 	 */
 	public void setProcessExternalEntities(boolean processExternalEntities) {
 		this.processExternalEntities = processExternalEntities;
+		if (processExternalEntities) {
+			setSupportDtd(true);
+		}
 	}
 
 	/**
@@ -111,6 +133,8 @@ public abstract class AbstractMarshaller implements Marshaller, Unmarshaller {
 		DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
 		factory.setValidating(false);
 		factory.setNamespaceAware(true);
+		factory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", !isSupportDtd());
+		factory.setFeature("http://xml.org/sax/features/external-general-entities", isProcessExternalEntities());
 		return factory;
 	}
 
@@ -125,7 +149,11 @@ public abstract class AbstractMarshaller implements Marshaller, Unmarshaller {
 	protected DocumentBuilder createDocumentBuilder(DocumentBuilderFactory factory)
 			throws ParserConfigurationException {
 
-		return factory.newDocumentBuilder();
+		DocumentBuilder documentBuilder = factory.newDocumentBuilder();
+		if (!isProcessExternalEntities()) {
+			documentBuilder.setEntityResolver(NO_OP_ENTITY_RESOLVER);
+		}
+		return documentBuilder;
 	}
 
 	/**
@@ -135,6 +163,7 @@ public abstract class AbstractMarshaller implements Marshaller, Unmarshaller {
 	 */
 	protected XMLReader createXmlReader() throws SAXException {
 		XMLReader xmlReader = XMLReaderFactory.createXMLReader();
+		xmlReader.setFeature("http://apache.org/xml/features/disallow-doctype-decl", !isSupportDtd());
 		xmlReader.setFeature("http://xml.org/sax/features/external-general-entities", isProcessExternalEntities());
 		if (!isProcessExternalEntities()) {
 			xmlReader.setEntityResolver(NO_OP_ENTITY_RESOLVER);
@@ -343,7 +372,17 @@ public abstract class AbstractMarshaller implements Marshaller, Unmarshaller {
 						"Could not create document placeholder for DOMSource: " + ex.getMessage(), ex);
 			}
 		}
-		return unmarshalDomNode(domSource.getNode());
+		try {
+			return unmarshalDomNode(domSource.getNode());
+		}
+		catch (NullPointerException ex) {
+			if (!isSupportDtd()) {
+				throw new UnmarshallingFailureException("NPE while unmarshalling. " +
+						"This can happen on JDK 1.6 due to the presence of DTD " +
+						"declarations, which are disabled.", ex);
+			}
+			throw ex;
+		}
 	}
 
 	/**
@@ -391,7 +430,17 @@ public abstract class AbstractMarshaller implements Marshaller, Unmarshaller {
 		if (saxSource.getInputSource() == null) {
 			saxSource.setInputSource(new InputSource());
 		}
-		return unmarshalSaxReader(saxSource.getXMLReader(), saxSource.getInputSource());
+		try {
+			return unmarshalSaxReader(saxSource.getXMLReader(), saxSource.getInputSource());
+		}
+		catch (NullPointerException ex) {
+			if (!isSupportDtd()) {
+				throw new UnmarshallingFailureException("NPE while unmarshalling. " +
+						"This can happen on JDK 1.6 due to the presence of DTD " +
+						"declarations, which are disabled.", ex);
+			}
+			throw ex;
+		}
 	}
 
 	/**
@@ -404,7 +453,7 @@ public abstract class AbstractMarshaller implements Marshaller, Unmarshaller {
 	 */
 	protected Object unmarshalStreamSource(StreamSource streamSource) throws XmlMappingException, IOException {
 		if (streamSource.getInputStream() != null) {
-			if (isProcessExternalEntities()) {
+			if (isProcessExternalEntities() && isSupportDtd()) {
 				return unmarshalInputStream(streamSource.getInputStream());
 			}
 			else {
@@ -414,7 +463,7 @@ public abstract class AbstractMarshaller implements Marshaller, Unmarshaller {
 			}
 		}
 		else if (streamSource.getReader() != null) {
-			if (isProcessExternalEntities()) {
+			if (isProcessExternalEntities() && isSupportDtd()) {
 				return unmarshalReader(streamSource.getReader());
 			}
 			else {

spring-oxm/src/test/java/org/springframework/oxm/castor/CastorUnmarshallerTests.java

@@ -206,21 +206,24 @@ public class CastorUnmarshallerTests extends AbstractUnmarshallerTests {
 			}
 		};
 
-		// 1. external-general-entities disabled (default)
+		// 1. external-general-entities and dtd support disabled (default)
 		marshaller.unmarshal(new StreamSource("1"));
 		assertNotNull(result.get());
+		assertEquals(true, result.get().getFeature("http://apache.org/xml/features/disallow-doctype-decl"));
 		assertEquals(false, result.get().getFeature("http://xml.org/sax/features/external-general-entities"));
 
-		// 2. external-general-entities disabled (default)
+		// 2. external-general-entities and dtd support enabled
 		result.set(null);
+		marshaller.setSupportDtd(true);
 		marshaller.setProcessExternalEntities(true);
 		marshaller.unmarshal(new StreamSource("1"));
 		assertNotNull(result.get());
+		assertEquals(false, result.get().getFeature("http://apache.org/xml/features/disallow-doctype-decl"));
 		assertEquals(true, result.get().getFeature("http://xml.org/sax/features/external-general-entities"));
 	}
 
 	@Test
-	public void unmarshalSaxSourceExternalEntities() throws Exception {
+	public void unmarshalSaxSourceWithXmlOptions() throws Exception {
 		final AtomicReference<XMLReader> result = new AtomicReference<XMLReader>();
 		CastorMarshaller marshaller = new CastorMarshaller() {
 			@Override
@@ -230,16 +233,19 @@ public class CastorUnmarshallerTests extends AbstractUnmarshallerTests {
 			}
 		};
 
-		// 1. external-general-entities disabled (default)
+		// 1. external-general-entities and dtd support disabled (default)
 		marshaller.unmarshal(new SAXSource(new InputSource("1")));
 		assertNotNull(result.get());
+		assertEquals(true, result.get().getFeature("http://apache.org/xml/features/disallow-doctype-decl"));
 		assertEquals(false, result.get().getFeature("http://xml.org/sax/features/external-general-entities"));
 
-		// 2. external-general-entities disabled (default)
+		// 2. external-general-entities and dtd support enabled
 		result.set(null);
+		marshaller.setSupportDtd(true);
 		marshaller.setProcessExternalEntities(true);
 		marshaller.unmarshal(new SAXSource(new InputSource("1")));
 		assertNotNull(result.get());
+		assertEquals(false, result.get().getFeature("http://apache.org/xml/features/disallow-doctype-decl"));
 		assertEquals(true, result.get().getFeature("http://xml.org/sax/features/external-general-entities"));
 	}
 

spring-oxm/src/test/java/org/springframework/oxm/jaxb/Jaxb2MarshallerTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2014 the original author or authors.
+ * Copyright 2002-2015 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -308,7 +308,7 @@ public class Jaxb2MarshallerTests extends AbstractMarshallerTests {
 	// SPR-10806
 
 	@Test
-	public void unmarshalStreamSourceExternalEntities() throws Exception {
+	public void unmarshalStreamSourceWithXmlOptions() throws Exception {
 
 		final javax.xml.bind.Unmarshaller unmarshaller = mock(javax.xml.bind.Unmarshaller.class);
 		Jaxb2Marshaller marshaller = new Jaxb2Marshaller() {
@@ -318,24 +318,27 @@ public class Jaxb2MarshallerTests extends AbstractMarshallerTests {
 			}
 		};
 
-		// 1. external-general-entities disabled (default)
+		// 1. external-general-entities and dtd support disabled (default)
 
 		marshaller.unmarshal(new StreamSource("1"));
 		ArgumentCaptor<SAXSource> sourceCaptor = ArgumentCaptor.forClass(SAXSource.class);
 		verify(unmarshaller).unmarshal(sourceCaptor.capture());
 
 		SAXSource result = sourceCaptor.getValue();
+		assertEquals(true, result.getXMLReader().getFeature("http://apache.org/xml/features/disallow-doctype-decl"));
 		assertEquals(false, result.getXMLReader().getFeature("http://xml.org/sax/features/external-general-entities"));
 
-		// 2. external-general-entities enabled
+		// 2. external-general-entities and dtd support enabled
 
 		reset(unmarshaller);
+		marshaller.setSupportDtd(true);
 		marshaller.setProcessExternalEntities(true);
 
 		marshaller.unmarshal(new StreamSource("1"));
 		verify(unmarshaller).unmarshal(sourceCaptor.capture());
 
 		result = sourceCaptor.getValue();
+		assertEquals(false, result.getXMLReader().getFeature("http://apache.org/xml/features/disallow-doctype-decl"));
 		assertEquals(true, result.getXMLReader().getFeature("http://xml.org/sax/features/external-general-entities"));
 	}
 
@@ -352,24 +355,27 @@ public class Jaxb2MarshallerTests extends AbstractMarshallerTests {
 			}
 		};
 
-		// 1. external-general-entities disabled (default)
+		// 1. external-general-entities and dtd support disabled (default)
 
 		marshaller.unmarshal(new SAXSource(new InputSource("1")));
 		ArgumentCaptor<SAXSource> sourceCaptor = ArgumentCaptor.forClass(SAXSource.class);
 		verify(unmarshaller).unmarshal(sourceCaptor.capture());
 
 		SAXSource result = sourceCaptor.getValue();
+		assertEquals(true, result.getXMLReader().getFeature("http://apache.org/xml/features/disallow-doctype-decl"));
 		assertEquals(false, result.getXMLReader().getFeature("http://xml.org/sax/features/external-general-entities"));
 
-		// 2. external-general-entities enabled
+		// 2. external-general-entities and dtd support enabled
 
 		reset(unmarshaller);
+		marshaller.setSupportDtd(true);
 		marshaller.setProcessExternalEntities(true);
 
 		marshaller.unmarshal(new SAXSource(new InputSource("1")));
 		verify(unmarshaller).unmarshal(sourceCaptor.capture());
 
 		result = sourceCaptor.getValue();
+		assertEquals(false, result.getXMLReader().getFeature("http://apache.org/xml/features/disallow-doctype-decl"));
 		assertEquals(true, result.getXMLReader().getFeature("http://xml.org/sax/features/external-general-entities"));
 	}
 

spring-web/src/main/java/org/springframework/http/converter/xml/Jaxb2CollectionHttpMessageConverter.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2014 the original author or authors.
+ * Copyright 2002-2015 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -226,6 +226,7 @@ public class Jaxb2CollectionHttpMessageConverter<T extends Collection>
 	 */
 	protected XMLInputFactory createXmlInputFactory() {
 		XMLInputFactory inputFactory = XMLInputFactory.newInstance();
+		inputFactory.setProperty(XMLInputFactory.SUPPORT_DTD, false);
 		inputFactory.setProperty(XMLInputFactory.IS_SUPPORTING_EXTERNAL_ENTITIES, false);
 		inputFactory.setXMLResolver(NO_OP_XML_RESOLVER);
 		return inputFactory;

spring-web/src/main/java/org/springframework/http/converter/xml/Jaxb2RootElementHttpMessageConverter.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2014 the original author or authors.
+ * Copyright 2002-2015 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -60,15 +60,37 @@ import org.springframework.util.ClassUtils;
  */
 public class Jaxb2RootElementHttpMessageConverter extends AbstractJaxb2HttpMessageConverter<Object> {
 
+	private boolean supportDtd = false;
+
 	private boolean processExternalEntities = false;
 
 
+	/**
+	 * Indicates whether DTD parsing should be supported.
+	 * <p>Default is {@code false} meaning that DTD is disabled.
+	 */
+	public void setSupportDtd(boolean supportDtd) {
+		this.supportDtd = supportDtd;
+	}
+
+	/**
+	 * Whether DTD parsing is supported.
+	 */
+	public boolean isSupportDtd() {
+		return this.supportDtd;
+	}
+
 	/**
 	 * Indicates whether external XML entities are processed when converting to a Source.
 	 * <p>Default is {@code false}, meaning that external entities are not resolved.
+	 * <p><strong>Note:</strong> setting this option to {@code true} also
+	 * automatically sets {@link #setSupportDtd} to {@code true}.
 	 */
 	public void setProcessExternalEntities(boolean processExternalEntities) {
 		this.processExternalEntities = processExternalEntities;
+		if (processExternalEntities) {
+			setSupportDtd(true);
+		}
 	}
 
 	/**
@@ -109,6 +131,14 @@ public class Jaxb2RootElementHttpMessageConverter extends AbstractJaxb2HttpMessa
 				return jaxbElement.getValue();
 			}
 		}
+		catch (NullPointerException ex) {
+			if (!isSupportDtd()) {
+				throw new HttpMessageNotReadableException("NPE while unmarshalling. " +
+						"This can happen on JDK 1.6 due to the presence of DTD " +
+						"declarations, which are disabled.", ex);
+			}
+			throw ex;
+		}
 		catch (UnmarshalException ex) {
 			throw new HttpMessageNotReadableException("Could not unmarshal to [" + clazz + "]: " + ex.getMessage(), ex);
 
@@ -124,6 +154,7 @@ public class Jaxb2RootElementHttpMessageConverter extends AbstractJaxb2HttpMessa
 			InputSource inputSource = new InputSource(streamSource.getInputStream());
 			try {
 				XMLReader xmlReader = XMLReaderFactory.createXMLReader();
+				xmlReader.setFeature("http://apache.org/xml/features/disallow-doctype-decl", !isSupportDtd());
 				String featureName = "http://xml.org/sax/features/external-general-entities";
 				xmlReader.setFeature(featureName, isProcessExternalEntities());
 				if (!isProcessExternalEntities()) {

spring-web/src/main/java/org/springframework/http/converter/xml/SourceHttpMessageConverter.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2014 the original author or authors.
+ * Copyright 2002-2015 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -74,6 +74,8 @@ public class SourceHttpMessageConverter<T extends Source> extends AbstractHttpMe
 
 	private final TransformerFactory transformerFactory = TransformerFactory.newInstance();
 
+	private boolean supportDtd = false;
+
 	private boolean processExternalEntities = false;
 
 
@@ -86,12 +88,32 @@ public class SourceHttpMessageConverter<T extends Source> extends AbstractHttpMe
 	}
 
 
+	/**
+	 * Indicates whether DTD parsing should be supported.
+	 * <p>Default is {@code false} meaning that DTD is disabled.
+	 */
+	public void setSupportDtd(boolean supportDtd) {
+		this.supportDtd = supportDtd;
+	}
+
+	/**
+	 * Whether DTD parsing is supported.
+	 */
+	public boolean isSupportDtd() {
+		return this.supportDtd;
+	}
+
 	/**
 	 * Indicates whether external XML entities are processed when converting to a Source.
 	 * <p>Default is {@code false}, meaning that external entities are not resolved.
+	 * <p><strong>Note:</strong> setting this option to {@code true} also
+	 * automatically sets {@link #setSupportDtd} to {@code true}.
 	 */
 	public void setProcessExternalEntities(boolean processExternalEntities) {
 		this.processExternalEntities = processExternalEntities;
+		if (processExternalEntities) {
+			setSupportDtd(true);
+		}
 	}
 
 	/**
@@ -132,6 +154,8 @@ public class SourceHttpMessageConverter<T extends Source> extends AbstractHttpMe
 		try {
 			DocumentBuilderFactory documentBuilderFactory = DocumentBuilderFactory.newInstance();
 			documentBuilderFactory.setNamespaceAware(true);
+			documentBuilderFactory.setFeature(
+					"http://apache.org/xml/features/disallow-doctype-decl", !isSupportDtd());
 			documentBuilderFactory.setFeature(
 					"http://xml.org/sax/features/external-general-entities", isProcessExternalEntities());
 			DocumentBuilder documentBuilder = documentBuilderFactory.newDocumentBuilder();
@@ -141,6 +165,14 @@ public class SourceHttpMessageConverter<T extends Source> extends AbstractHttpMe
 			Document document = documentBuilder.parse(body);
 			return new DOMSource(document);
 		}
+		catch (NullPointerException ex) {
+			if (!isSupportDtd()) {
+				throw new HttpMessageNotReadableException("NPE while unmarshalling. " +
+						"This can happen on JDK 1.6 due to the presence of DTD " +
+						"declarations, which are disabled.", ex);
+			}
+			throw ex;
+		}
 		catch (ParserConfigurationException ex) {
 			throw new HttpMessageNotReadableException("Could not set feature: " + ex.getMessage(), ex);
 		}
@@ -152,11 +184,12 @@ public class SourceHttpMessageConverter<T extends Source> extends AbstractHttpMe
 	private SAXSource readSAXSource(InputStream body) throws IOException {
 		try {
 			XMLReader reader = XMLReaderFactory.createXMLReader();
+			reader.setFeature("http://apache.org/xml/features/disallow-doctype-decl", !isSupportDtd());
 			reader.setFeature("http://xml.org/sax/features/external-general-entities", isProcessExternalEntities());
-			byte[] bytes = StreamUtils.copyToByteArray(body);
 			if (!isProcessExternalEntities()) {
 				reader.setEntityResolver(NO_OP_ENTITY_RESOLVER);
 			}
+			byte[] bytes = StreamUtils.copyToByteArray(body);
 			return new SAXSource(reader, new InputSource(new ByteArrayInputStream(bytes)));
 		}
 		catch (SAXException ex) {

spring-web/src/test/java/org/springframework/http/converter/xml/Jaxb2CollectionHttpMessageConverterTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2012 the original author or authors.
+ * Copyright 2002-2015 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -28,11 +28,18 @@ import javax.xml.bind.annotation.XmlAttribute;
 import javax.xml.bind.annotation.XmlElement;
 import javax.xml.bind.annotation.XmlRootElement;
 import javax.xml.bind.annotation.XmlType;
+import javax.xml.stream.XMLInputFactory;
 
 import org.junit.Before;
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.ExpectedException;
+
 import org.springframework.core.ParameterizedTypeReference;
+import org.springframework.core.io.ClassPathResource;
+import org.springframework.core.io.Resource;
 import org.springframework.http.MockHttpInputMessage;
+import org.springframework.http.converter.HttpMessageNotReadableException;
 
 /**
  * Test fixture for {@link Jaxb2CollectionHttpMessageConverter}.
@@ -51,10 +58,13 @@ public class Jaxb2CollectionHttpMessageConverterTests {
 
 	private Type typeSetType;
 
+	@Rule
+	public ExpectedException thrown = ExpectedException.none();
+
 
 	@Before
 	public void setUp() {
-		converter = new Jaxb2CollectionHttpMessageConverter<Collection<Object>>();
+		converter = new Jaxb2CollectionHttpMessageConverter<>();
 		rootElementListType = new ParameterizedTypeReference<List<RootElement>>() {}.getType();
 		rootElementSetType = new ParameterizedTypeReference<Set<RootElement>>() {}.getType();
 		typeListType = new ParameterizedTypeReference<List<TestType>>() {}.getType();
@@ -120,6 +130,82 @@ public class Jaxb2CollectionHttpMessageConverterTests {
 		assertTrue("Invalid result", result.contains(new TestType("2")));
 	}
 
+	@Test
+	@SuppressWarnings("unchecked")
+	public void readXmlRootElementExternalEntityDisabled() throws Exception {
+
+		Resource external = new ClassPathResource("external.txt", getClass());
+		String content =  "<!DOCTYPE root [" +
+				"  <!ELEMENT external ANY >\n" +
+				"  <!ENTITY ext SYSTEM \"" + external.getURI() + "\" >]>" +
+				"  <list><rootElement><type s=\"1\"/><external>&ext;</external></rootElement></list>";
+		MockHttpInputMessage inputMessage = new MockHttpInputMessage(content.getBytes("UTF-8"));
+
+		converter = new Jaxb2CollectionHttpMessageConverter<Collection<Object>>() {
+
+			@Override
+			protected XMLInputFactory createXmlInputFactory() {
+				XMLInputFactory inputFactory = super.createXmlInputFactory();
+				inputFactory.setProperty(XMLInputFactory.SUPPORT_DTD, true);
+				return inputFactory;
+			}
+		};
+
+		Collection<RootElement> result = converter.read(rootElementListType, null, inputMessage);
+		assertEquals(1, result.size());
+		assertEquals("", result.iterator().next().external);
+	}
+
+	@Test
+	@SuppressWarnings("unchecked")
+	public void readXmlRootElementExternalEntityEnabled() throws Exception {
+
+		Resource external = new ClassPathResource("external.txt", getClass());
+		String content =  "<!DOCTYPE root [" +
+				"  <!ELEMENT external ANY >\n" +
+				"  <!ENTITY ext SYSTEM \"" + external.getURI() + "\" >]>" +
+				"  <list><rootElement><type s=\"1\"/><external>&ext;</external></rootElement></list>";
+		MockHttpInputMessage inputMessage = new MockHttpInputMessage(content.getBytes("UTF-8"));
+
+		Jaxb2CollectionHttpMessageConverter<?> c = new Jaxb2CollectionHttpMessageConverter<Collection<Object>>() {
+			@Override
+			protected XMLInputFactory createXmlInputFactory() {
+				XMLInputFactory inputFactory = XMLInputFactory.newInstance();
+				inputFactory.setProperty(XMLInputFactory.IS_REPLACING_ENTITY_REFERENCES, true);
+				return inputFactory;
+			}
+		};
+
+		Collection<RootElement> result = c.read(rootElementListType, null, inputMessage);
+		assertEquals(1, result.size());
+		assertEquals("Foo Bar", result.iterator().next().external);
+	}
+
+	@Test
+	public void testXmlBomb() throws Exception {
+		// https://en.wikipedia.org/wiki/Billion_laughs
+		// https://msdn.microsoft.com/en-us/magazine/ee335713.aspx
+		String content = "<?xml version=\"1.0\"?>\n" +
+				"<!DOCTYPE lolz [\n" +
+				" <!ENTITY lol \"lol\">\n" +
+				" <!ELEMENT lolz (#PCDATA)>\n" +
+				" <!ENTITY lol1 \"&lol;&lol;&lol;&lol;&lol;&lol;&lol;&lol;&lol;&lol;\">\n" +
+				" <!ENTITY lol2 \"&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;\">\n" +
+				" <!ENTITY lol3 \"&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;\">\n" +
+				" <!ENTITY lol4 \"&lol3;&lol3;&lol3;&lol3;&lol3;&lol3;&lol3;&lol3;&lol3;&lol3;\">\n" +
+				" <!ENTITY lol5 \"&lol4;&lol4;&lol4;&lol4;&lol4;&lol4;&lol4;&lol4;&lol4;&lol4;\">\n" +
+				" <!ENTITY lol6 \"&lol5;&lol5;&lol5;&lol5;&lol5;&lol5;&lol5;&lol5;&lol5;&lol5;\">\n" +
+				" <!ENTITY lol7 \"&lol6;&lol6;&lol6;&lol6;&lol6;&lol6;&lol6;&lol6;&lol6;&lol6;\">\n" +
+				" <!ENTITY lol8 \"&lol7;&lol7;&lol7;&lol7;&lol7;&lol7;&lol7;&lol7;&lol7;&lol7;\">\n" +
+				" <!ENTITY lol9 \"&lol8;&lol8;&lol8;&lol8;&lol8;&lol8;&lol8;&lol8;&lol8;&lol8;\">\n" +
+				"]>\n" +
+				"<list><rootElement><external>&lol9;</external></rootElement></list>";
+		MockHttpInputMessage inputMessage = new MockHttpInputMessage(content.getBytes("UTF-8"));
+		this.thrown.expect(HttpMessageNotReadableException.class);
+		this.thrown.expectMessage("\"lol9\"");
+		this.converter.read(this.rootElementListType, null, inputMessage);
+	}
+
 
 	@XmlRootElement
 	public static class RootElement {
@@ -134,6 +220,9 @@ public class Jaxb2CollectionHttpMessageConverterTests {
 		@XmlElement
 		public TestType type = new TestType();
 
+		@XmlElement(required=false)
+		public String external;
+
 		@Override
 		public boolean equals(Object o) {
 			if (this == o) {

spring-web/src/test/java/org/springframework/http/converter/xml/Jaxb2RootElementHttpMessageConverterTest.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2012 the original author or authors.
+ * Copyright 2002-2015 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -27,7 +27,9 @@ import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 import org.junit.Before;
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.ExpectedException;
 
 import org.springframework.aop.framework.AdvisedSupport;
 import org.springframework.aop.framework.AopProxy;
@@ -37,6 +39,7 @@ import org.springframework.core.io.Resource;
 import org.springframework.http.MediaType;
 import org.springframework.http.MockHttpInputMessage;
 import org.springframework.http.MockHttpOutputMessage;
+import org.springframework.http.converter.HttpMessageNotReadableException;
 
 /** @author Arjen Poutsma */
 public class Jaxb2RootElementHttpMessageConverterTest {
@@ -47,6 +50,10 @@ public class Jaxb2RootElementHttpMessageConverterTest {
 
 	private RootElement rootElementCglib;
 
+	@Rule
+	public ExpectedException thrown = ExpectedException.none();
+
+
 	@Before
 	public void setUp() {
 		converter = new Jaxb2RootElementHttpMessageConverter();
@@ -105,6 +112,7 @@ public class Jaxb2RootElementHttpMessageConverterTest {
 				"  <!ENTITY ext SYSTEM \"" + external.getURI() + "\" >]>" +
 				"  <rootElement><external>&ext;</external></rootElement>";
 		MockHttpInputMessage inputMessage = new MockHttpInputMessage(content.getBytes("UTF-8"));
+		converter.setSupportDtd(true);
 		RootElement rootElement = (RootElement) converter.read(RootElement.class, inputMessage);
 
 		assertEquals("", rootElement.external);
@@ -124,6 +132,31 @@ public class Jaxb2RootElementHttpMessageConverterTest {
 		assertEquals("Foo Bar", rootElement.external);
 	}
 
+	@Test
+	public void testXmlBomb() throws Exception {
+		// https://en.wikipedia.org/wiki/Billion_laughs
+		// https://msdn.microsoft.com/en-us/magazine/ee335713.aspx
+		String content = "<?xml version=\"1.0\"?>\n" +
+				"<!DOCTYPE lolz [\n" +
+				" <!ENTITY lol \"lol\">\n" +
+				" <!ELEMENT lolz (#PCDATA)>\n" +
+				" <!ENTITY lol1 \"&lol;&lol;&lol;&lol;&lol;&lol;&lol;&lol;&lol;&lol;\">\n" +
+				" <!ENTITY lol2 \"&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;\">\n" +
+				" <!ENTITY lol3 \"&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;\">\n" +
+				" <!ENTITY lol4 \"&lol3;&lol3;&lol3;&lol3;&lol3;&lol3;&lol3;&lol3;&lol3;&lol3;\">\n" +
+				" <!ENTITY lol5 \"&lol4;&lol4;&lol4;&lol4;&lol4;&lol4;&lol4;&lol4;&lol4;&lol4;\">\n" +
+				" <!ENTITY lol6 \"&lol5;&lol5;&lol5;&lol5;&lol5;&lol5;&lol5;&lol5;&lol5;&lol5;\">\n" +
+				" <!ENTITY lol7 \"&lol6;&lol6;&lol6;&lol6;&lol6;&lol6;&lol6;&lol6;&lol6;&lol6;\">\n" +
+				" <!ENTITY lol8 \"&lol7;&lol7;&lol7;&lol7;&lol7;&lol7;&lol7;&lol7;&lol7;&lol7;\">\n" +
+				" <!ENTITY lol9 \"&lol8;&lol8;&lol8;&lol8;&lol8;&lol8;&lol8;&lol8;&lol8;&lol8;\">\n" +
+				"]>\n" +
+				"<rootElement><external>&lol9;</external></rootElement>";
+		MockHttpInputMessage inputMessage = new MockHttpInputMessage(content.getBytes("UTF-8"));
+		this.thrown.expect(HttpMessageNotReadableException.class);
+		this.thrown.expectMessage("DOCTYPE is disallowed");
+		this.converter.read(RootElement.class, inputMessage);
+	}
+
 	@Test
 	public void writeXmlRootElement() throws Exception {
 		MockHttpOutputMessage outputMessage = new MockHttpOutputMessage();

spring-web/src/test/java/org/springframework/http/converter/xml/SourceHttpMessageConverterTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2014 the original author or authors.
+ * Copyright 2002-2015 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -27,7 +27,9 @@ import javax.xml.transform.sax.SAXSource;
 import javax.xml.transform.stream.StreamSource;
 
 import org.junit.Before;
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.ExpectedException;
 import org.w3c.dom.Document;
 import org.w3c.dom.Element;
 import org.xml.sax.InputSource;
@@ -40,6 +42,7 @@ import org.springframework.core.io.Resource;
 import org.springframework.http.MediaType;
 import org.springframework.http.MockHttpInputMessage;
 import org.springframework.http.MockHttpOutputMessage;
+import org.springframework.http.converter.HttpMessageNotReadableException;
 import org.springframework.util.FileCopyUtils;
 
 import static org.custommonkey.xmlunit.XMLAssert.*;
@@ -58,6 +61,9 @@ public class SourceHttpMessageConverterTests {
 
 	private String bodyExternal;
 
+	@Rule
+	public ExpectedException thrown = ExpectedException.none();
+
 
 	@Before
 	public void setUp() throws IOException {
@@ -95,12 +101,40 @@ public class SourceHttpMessageConverterTests {
 	public void readDOMSourceExternal() throws Exception {
 		MockHttpInputMessage inputMessage = new MockHttpInputMessage(bodyExternal.getBytes("UTF-8"));
 		inputMessage.getHeaders().setContentType(new MediaType("application", "xml"));
+		converter.setSupportDtd(true);
 		DOMSource result = (DOMSource) converter.read(DOMSource.class, inputMessage);
 		Document document = (Document) result.getNode();
 		assertEquals("Invalid result", "root", document.getDocumentElement().getLocalName());
 		assertNotEquals("Invalid result", "Foo Bar", document.getDocumentElement().getTextContent());
 	}
 
+	@Test
+	public void readDomSourceWithXmlBomb() throws Exception {
+		// https://en.wikipedia.org/wiki/Billion_laughs
+		// https://msdn.microsoft.com/en-us/magazine/ee335713.aspx
+		String content = "<?xml version=\"1.0\"?>\n" +
+				"<!DOCTYPE lolz [\n" +
+				" <!ENTITY lol \"lol\">\n" +
+				" <!ELEMENT lolz (#PCDATA)>\n" +
+				" <!ENTITY lol1 \"&lol;&lol;&lol;&lol;&lol;&lol;&lol;&lol;&lol;&lol;\">\n" +
+				" <!ENTITY lol2 \"&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;\">\n" +
+				" <!ENTITY lol3 \"&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;\">\n" +
+				" <!ENTITY lol4 \"&lol3;&lol3;&lol3;&lol3;&lol3;&lol3;&lol3;&lol3;&lol3;&lol3;\">\n" +
+				" <!ENTITY lol5 \"&lol4;&lol4;&lol4;&lol4;&lol4;&lol4;&lol4;&lol4;&lol4;&lol4;\">\n" +
+				" <!ENTITY lol6 \"&lol5;&lol5;&lol5;&lol5;&lol5;&lol5;&lol5;&lol5;&lol5;&lol5;\">\n" +
+				" <!ENTITY lol7 \"&lol6;&lol6;&lol6;&lol6;&lol6;&lol6;&lol6;&lol6;&lol6;&lol6;\">\n" +
+				" <!ENTITY lol8 \"&lol7;&lol7;&lol7;&lol7;&lol7;&lol7;&lol7;&lol7;&lol7;&lol7;\">\n" +
+				" <!ENTITY lol9 \"&lol8;&lol8;&lol8;&lol8;&lol8;&lol8;&lol8;&lol8;&lol8;&lol8;\">\n" +
+				"]>\n" +
+				"<root>&lol9;</root>";
+		MockHttpInputMessage inputMessage = new MockHttpInputMessage(content.getBytes("UTF-8"));
+
+		this.thrown.expect(HttpMessageNotReadableException.class);
+		this.thrown.expectMessage("DOCTYPE is disallowed");
+
+		this.converter.read(DOMSource.class, inputMessage);
+	}
+
 	@Test
 	public void readSAXSource() throws Exception {
 		MockHttpInputMessage inputMessage = new MockHttpInputMessage(BODY.getBytes("UTF-8"));
@@ -115,6 +149,7 @@ public class SourceHttpMessageConverterTests {
 	public void readSAXSourceExternal() throws Exception {
 		MockHttpInputMessage inputMessage = new MockHttpInputMessage(bodyExternal.getBytes("UTF-8"));
 		inputMessage.getHeaders().setContentType(new MediaType("application", "xml"));
+		converter.setSupportDtd(true);
 		SAXSource result = (SAXSource) converter.read(SAXSource.class, inputMessage);
 		InputSource inputSource = result.getInputSource();
 		XMLReader reader = result.getXMLReader();
@@ -128,6 +163,37 @@ public class SourceHttpMessageConverterTests {
 		reader.parse(inputSource);
 	}
 
+	@Test
+	public void readSAXSourceWithXmlBomb() throws Exception {
+		// https://en.wikipedia.org/wiki/Billion_laughs
+		// https://msdn.microsoft.com/en-us/magazine/ee335713.aspx
+		String content = "<?xml version=\"1.0\"?>\n" +
+				"<!DOCTYPE lolz [\n" +
+				" <!ENTITY lol \"lol\">\n" +
+				" <!ELEMENT lolz (#PCDATA)>\n" +
+				" <!ENTITY lol1 \"&lol;&lol;&lol;&lol;&lol;&lol;&lol;&lol;&lol;&lol;\">\n" +
+				" <!ENTITY lol2 \"&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;\">\n" +
+				" <!ENTITY lol3 \"&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;\">\n" +
+				" <!ENTITY lol4 \"&lol3;&lol3;&lol3;&lol3;&lol3;&lol3;&lol3;&lol3;&lol3;&lol3;\">\n" +
+				" <!ENTITY lol5 \"&lol4;&lol4;&lol4;&lol4;&lol4;&lol4;&lol4;&lol4;&lol4;&lol4;\">\n" +
+				" <!ENTITY lol6 \"&lol5;&lol5;&lol5;&lol5;&lol5;&lol5;&lol5;&lol5;&lol5;&lol5;\">\n" +
+				" <!ENTITY lol7 \"&lol6;&lol6;&lol6;&lol6;&lol6;&lol6;&lol6;&lol6;&lol6;&lol6;\">\n" +
+				" <!ENTITY lol8 \"&lol7;&lol7;&lol7;&lol7;&lol7;&lol7;&lol7;&lol7;&lol7;&lol7;\">\n" +
+				" <!ENTITY lol9 \"&lol8;&lol8;&lol8;&lol8;&lol8;&lol8;&lol8;&lol8;&lol8;&lol8;\">\n" +
+				"]>\n" +
+				"<root>&lol9;</root>";
+
+		MockHttpInputMessage inputMessage = new MockHttpInputMessage(content.getBytes("UTF-8"));
+		SAXSource result = (SAXSource) this.converter.read(SAXSource.class, inputMessage);
+
+		this.thrown.expect(SAXException.class);
+		this.thrown.expectMessage("DOCTYPE is disallowed");
+
+		InputSource inputSource = result.getInputSource();
+		XMLReader reader = result.getXMLReader();
+		reader.parse(inputSource);
+	}
+
 	@Test
 	public void readStreamSource() throws Exception {
 		MockHttpInputMessage inputMessage = new MockHttpInputMessage(BODY.getBytes("UTF-8"));

spring-web/src/test/resources/org/springframework/http/converter/xml/external.txt

@@ -1 +1 @@
-Foo Bar
+Foo Bar