Spring/spring-framework
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Disable DTD when parsing untrusted XML input

Browse Source 
Issue: SPR-13136
This commit is contained in:
Rossen Stoyanchev
2015-06-30 07:54:24 -04:00
parent a837cbe81f
commit 9c3580d04e
12 changed files with 476 additions and 56 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -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.
@@ -37,6 +37,7 @@ import java.util.Calendar;
import java.util.Date;
import java.util.Map;
import java.util.UUID;
import javax.activation.DataHandler;
import javax.activation.DataSource;
import javax.xml.XMLConstants;
@@ -177,6 +178,8 @@ public class Jaxb2Marshaller implements MimeMarshaller, MimeUnmarshaller, Generi
private Schema schema;
private boolean supportDtd = false;
private boolean processExternalEntities = false;
@@ -391,6 +394,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.
@@ -398,9 +416,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);
}
}
/**
@@ -410,6 +433,7 @@ public class Jaxb2Marshaller implements MimeMarshaller, MimeUnmarshaller, Generi
return this.processExternalEntities;
}
@Override
public void setBeanClassLoader(ClassLoader classLoader) {
this.beanClassLoader = classLoader;
@@ -754,6 +778,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);
}
@@ -809,6 +841,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()) {

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

@@ -72,6 +72,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;
@@ -79,6 +81,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.
@@ -86,9 +103,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);
}
}
/**
@@ -133,6 +155,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;
}
@@ -147,7 +171,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;
}
/**
@@ -157,6 +185,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 {
if (domSource.getNode() == null) {
domSource.setNode(buildDocument());
}
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.");
}
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 {

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

@@ -16,10 +16,18 @@
package org.springframework.oxm.castor;
import static junit.framework.Assert.assertEquals;
import static org.hamcrest.CoreMatchers.*;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertSame;
import static org.junit.Assert.*;
import java.io.ByteArrayInputStream;
import java.io.IOException;
import java.io.StringReader;
import java.util.concurrent.atomic.AtomicReference;
import javax.xml.transform.sax.SAXSource;
import javax.xml.transform.stream.StreamSource;
@@ -33,9 +41,6 @@ import org.springframework.oxm.AbstractUnmarshallerTests;
import org.springframework.oxm.MarshallingException;
import org.springframework.oxm.Unmarshaller;
import static org.hamcrest.CoreMatchers.*;
import static org.junit.Assert.*;
/**
* @author Arjen Poutsma
* @author Jakub Narloch
@@ -86,7 +91,7 @@ public class CastorUnmarshallerTests extends AbstractUnmarshallerTests {
@Test
public void unmarshalTargetClass() throws Exception {
CastorMarshaller unmarshaller = new CastorMarshaller();
unmarshaller.setTargetClasses(new Class[] { Flights.class } );
unmarshaller.setTargetClasses(new Class[] {Flights.class});
unmarshaller.afterPropertiesSet();
StreamSource source = new StreamSource(new ByteArrayInputStream(INPUT_STRING.getBytes("UTF-8")));
Object flights = unmarshaller.unmarshal(source);
@@ -97,7 +102,7 @@ public class CastorUnmarshallerTests extends AbstractUnmarshallerTests {
public void testSetBothTargetClassesAndMapping() throws IOException {
CastorMarshaller unmarshaller = new CastorMarshaller();
unmarshaller.setMappingLocation(new ClassPathResource("order-mapping.xml", CastorMarshaller.class));
unmarshaller.setTargetClasses(new Class[] { Order.class } );
unmarshaller.setTargetClasses(new Class[] {Order.class});
unmarshaller.afterPropertiesSet();
String xml = "<order>" +
@@ -183,7 +188,7 @@ public class CastorUnmarshallerTests extends AbstractUnmarshallerTests {
@Ignore("Fails on the build server for some reason")
public void testClearCollectionsFalse() throws Exception {
Flights flights = new Flights();
flights.setFlight(new Flight[]{new Flight(), null});
flights.setFlight(new Flight[] {new Flight(), null});
getCastorUnmarshaller().setRootObject(flights);
getCastorUnmarshaller().setClearCollections(false);
Object result = unmarshalFlights();
@@ -196,7 +201,7 @@ public class CastorUnmarshallerTests extends AbstractUnmarshallerTests {
}
@Test
public void unmarshalStreamSourceExternalEntities() throws Exception {
public void unmarshalStreamSourceWithXmlOptions() throws Exception {
final AtomicReference<XMLReader> result = new AtomicReference<XMLReader>();
CastorMarshaller marshaller = new CastorMarshaller() {
@Override
@@ -206,21 +211,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 +238,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"));
}

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

@@ -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.
@@ -310,7 +310,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() {
@@ -320,31 +320,34 @@ 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.setProcessExternalEntities(true);
marshaller.setSupportDtd(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"));
}
// SPR-10806
@Test
public void unmarshalSaxSourceExternalEntities() throws Exception {
public void unmarshalSaxSourceWithXmlOptions() throws Exception {
final javax.xml.bind.Unmarshaller unmarshaller = mock(javax.xml.bind.Unmarshaller.class);
Jaxb2Marshaller marshaller = new Jaxb2Marshaller() {
@@ -354,24 +357,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.setProcessExternalEntities(true);
marshaller.setSupportDtd(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"));
}