Disable DTD when parsing untrusted XML input

Issue: SPR-13136
This commit is contained in:
Rossen Stoyanchev
2015-06-30 07:50:21 -04:00
parent 2d6ec953e4
commit d79ec68db4
12 changed files with 476 additions and 56 deletions

View File

@@ -16,6 +16,7 @@
package org.springframework.http.converter.json;
import java.io.ByteArrayInputStream;
import java.text.DateFormat;
import java.text.SimpleDateFormat;
import java.util.Arrays;
@@ -27,6 +28,9 @@ import java.util.Locale;
import java.util.Map;
import java.util.TimeZone;
import javax.xml.stream.XMLInputFactory;
import javax.xml.stream.XMLResolver;
import com.fasterxml.jackson.annotation.JsonFilter;
import com.fasterxml.jackson.annotation.JsonInclude;
import com.fasterxml.jackson.core.JsonGenerator;
@@ -520,22 +524,15 @@ public class Jackson2ObjectMapperBuilder {
*/
@SuppressWarnings("unchecked")
public <T extends ObjectMapper> T build() {
ObjectMapper objectMapper;
ObjectMapper mapper;
if (this.createXmlMapper) {
try {
Class<? extends ObjectMapper> xmlMapper = (Class<? extends ObjectMapper>)
ClassUtils.forName("com.fasterxml.jackson.dataformat.xml.XmlMapper", this.moduleClassLoader);
objectMapper = BeanUtils.instantiate(xmlMapper);
}
catch (ClassNotFoundException ex) {
throw new IllegalStateException("Could not instantiate XmlMapper - not found on classpath");
}
mapper = new XmlObjectMapperInitializer().create();
}
else {
objectMapper = new ObjectMapper();
mapper = new ObjectMapper();
}
configure(objectMapper);
return (T) objectMapper;
configure(mapper);
return (T) mapper;
}
/**
@@ -721,4 +718,23 @@ public class Jackson2ObjectMapperBuilder {
return new Jackson2ObjectMapperBuilder().createXmlMapper(true);
}
private static class XmlObjectMapperInitializer {
public ObjectMapper create() {
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 new XmlMapper(inputFactory);
}
private static final XMLResolver NO_OP_XML_RESOLVER = new XMLResolver() {
@Override
public Object resolveEntity(String publicID, String systemID, String base, String ns) {
return new ByteArrayInputStream(new byte[0]);
}
};
}
}

View File

@@ -246,6 +246,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;

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.
@@ -18,6 +18,7 @@ package org.springframework.http.converter.xml;
import java.io.IOException;
import java.io.StringReader;
import javax.xml.bind.JAXBElement;
import javax.xml.bind.JAXBException;
import javax.xml.bind.MarshalException;
@@ -60,15 +61,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 +132,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 +155,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()) {

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.
@@ -23,6 +23,7 @@ import java.io.OutputStream;
import java.io.StringReader;
import java.util.HashSet;
import java.util.Set;
import javax.xml.parsers.DocumentBuilder;
import javax.xml.parsers.DocumentBuilderFactory;
import javax.xml.parsers.ParserConfigurationException;
@@ -79,6 +80,8 @@ public class SourceHttpMessageConverter<T extends Source> extends AbstractHttpMe
private final TransformerFactory transformerFactory = TransformerFactory.newInstance();
private boolean supportDtd = false;
private boolean processExternalEntities = false;
@@ -91,12 +94,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);
}
}
/**
@@ -140,6 +163,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();
@@ -149,6 +174,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);
}
@@ -160,11 +193,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) {
@@ -174,7 +208,8 @@ public class SourceHttpMessageConverter<T extends Source> extends AbstractHttpMe
private Source readStAXSource(InputStream body) {
try {
XMLInputFactory inputFactory = XMLInputFactory.newFactory();
XMLInputFactory inputFactory = XMLInputFactory.newInstance();
inputFactory.setProperty(XMLInputFactory.SUPPORT_DTD, isSupportDtd());
inputFactory.setProperty(XMLInputFactory.IS_SUPPORTING_EXTERNAL_ENTITIES, isProcessExternalEntities());
if (!isProcessExternalEntities()) {
inputFactory.setXMLResolver(NO_OP_XML_RESOLVER);