Severity: Important Vendor: Spring by Pivotal Versions Affected: - Spring MVC 3.0.0 to 3.2.4 - Spring MVC 4.0.0.M1-4.0.0.RC1 - Earlier unsupported versions may be affected Description: Spring MVC's SourceHttpMessageConverter also processed user provided XML and neither disabled XML external entities nor provided an option to disable them. SourceHttpMessageConverter has been modified to provide an option to control the processing of XML external entities and that processing is now disabled by default. Mitigation: Users of affected versions should apply the following mitigation: - Users of 3.x should upgrade to 3.2.5 or later - Users of 4.x should upgrade to 4.0.0 or later (This is also fixed in 4.0.0-RC2 but users are recommended to use 4.0.0 or later) Credit: This issue was identified by the Spring development team. References: http://www.gopivotal.com/security/cve-2013-6429 https://jira.springsource.org/browse/SPR-11078 https://github.com/spring-projects/spring-framework/commit/2ae6a6a3415eebc57babcb9d3e5505887eda6d8a http://www.gopivotal.com/security/cve-2013-4152 History: 2014-Jan-14: Initial vulnerability report published. spring-web/src/main/java/org/springframework/http/converter/xml/SourceHttpMessageConverter.java View file @ 2ae6a6a @@ -1,5 +1,5 @@ /* - * Copyright 2002-2011 the original author or authors. + * Copyright 2002-2013 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. @@ -17,73 +17,144 @@ package org.springframework.http.converter.xml; import java.io.ByteArrayInputStream; -import java.io.ByteArrayOutputStream; import java.io.IOException; +import java.io.InputStream; import java.io.OutputStream; +import javax.xml.parsers.DocumentBuilder; +import javax.xml.parsers.DocumentBuilderFactory; +import javax.xml.parsers.ParserConfigurationException; +import javax.xml.stream.XMLInputFactory; +import javax.xml.stream.XMLStreamException; +import javax.xml.stream.XMLStreamReader; import javax.xml.transform.Result; import javax.xml.transform.Source; import javax.xml.transform.TransformerException; -import javax.xml.transform.dom.DOMResult; +import javax.xml.transform.TransformerFactory; import javax.xml.transform.dom.DOMSource; import javax.xml.transform.sax.SAXSource; +import javax.xml.transform.stax.StAXSource; import javax.xml.transform.stream.StreamResult; import javax.xml.transform.stream.StreamSource; +import org.w3c.dom.Document; import org.xml.sax.InputSource; +import org.xml.sax.SAXException; +import org.xml.sax.XMLReader; +import org.xml.sax.helpers.XMLReaderFactory; -import org.springframework.http.HttpHeaders; +import org.springframework.http.HttpInputMessage; +import org.springframework.http.HttpOutputMessage; import org.springframework.http.MediaType; +import org.springframework.http.converter.AbstractHttpMessageConverter; import org.springframework.http.converter.HttpMessageConversionException; import org.springframework.http.converter.HttpMessageNotReadableException; import org.springframework.http.converter.HttpMessageNotWritableException; +import org.springframework.util.StreamUtils; /** - * Implementation of {@link org.springframework.http.converter.HttpMessageConverter} that can read and write {@link - * Source} objects. + * Implementation of {@link org.springframework.http.converter.HttpMessageConverter} + * that can read and write {@link Source} objects. * * @author Arjen Poutsma * @since 3.0 */ -public class SourceHttpMessageConverter<T extends Source> extends AbstractXmlHttpMessageConverter<T> { +public class SourceHttpMessageConverter<T extends Source> extends AbstractHttpMessageConverter<T> { + + private final TransformerFactory transformerFactory = TransformerFactory.newInstance(); + + private boolean processExternalEntities = false; + + /** + * Sets the {@link #setSupportedMediaTypes(java.util.List) supportedMediaTypes} + * to {@code text/xml} and {@code application/xml}, and {@code application/*-xml}. + */ + public SourceHttpMessageConverter() { + super(MediaType.APPLICATION_XML, MediaType.TEXT_XML, new MediaType("application", "*+xml")); + } + + + /** + * Indicates whether external XML entities are processed when converting + * to a Source. + * <p>Default is {@code false}, meaning that external entities are not resolved. + */ + public void setProcessExternalEntities(boolean processExternalEntities) { + this.processExternalEntities = processExternalEntities; + } @Override public boolean supports(Class<?> clazz) { - return DOMSource.class.equals(clazz) || SAXSource.class.equals(clazz) || StreamSource.class.equals(clazz) || - Source.class.equals(clazz); + return DOMSource.class.equals(clazz) || SAXSource.class.equals(clazz) + || StreamSource.class.equals(clazz) || Source.class.equals(clazz); } @Override - @SuppressWarnings("unchecked") - protected T readFromSource(Class clazz, HttpHeaders headers, Source source) throws IOException { + protected T readInternal(Class<? extends T> clazz, HttpInputMessage inputMessage) + throws IOException, HttpMessageNotReadableException { + + InputStream body = inputMessage.getBody(); + if (DOMSource.class.equals(clazz)) { + return (T) readDOMSource(body); + } + else if (SAXSource.class.equals(clazz)) { + return (T) readSAXSource(body); + } + else if (StAXSource.class.equals(clazz)) { + return (T) readStAXSource(body); + } + else if (StreamSource.class.equals(clazz) || Source.class.equals(clazz)) { + return (T) readStreamSource(body); + } + else { + throw new HttpMessageConversionException("Could not read class [" + clazz + + "]. Only DOMSource, SAXSource, and StreamSource are supported."); + } + } + + private DOMSource readDOMSource(InputStream body) throws IOException { try { - if (DOMSource.class.equals(clazz)) { - DOMResult domResult = new DOMResult(); - transform(source, domResult); - return (T) new DOMSource(domResult.getNode()); - } - else if (SAXSource.class.equals(clazz)) { - ByteArrayInputStream bis = transformToByteArrayInputStream(source); - return (T) new SAXSource(new InputSource(bis)); - } - else if (StreamSource.class.equals(clazz) || Source.class.equals(clazz)) { - ByteArrayInputStream bis = transformToByteArrayInputStream(source); - return (T) new StreamSource(bis); - } - else { - throw new HttpMessageConversionException("Could not read class [" + clazz + - "]. Only DOMSource, SAXSource, and StreamSource are supported."); - } + DocumentBuilderFactory documentBuilderFactory = DocumentBuilderFactory.newInstance(); + documentBuilderFactory.setNamespaceAware(true); + documentBuilderFactory.setFeature("http://xml.org/sax/features/external-general-entities", processExternalEntities); + DocumentBuilder documentBuilder = documentBuilderFactory.newDocumentBuilder(); + Document document = documentBuilder.parse(body); + return new DOMSource(document); } - catch (TransformerException ex) { - throw new HttpMessageNotReadableException("Could not transform from [" + source + "] to [" + clazz + "]", - ex); + catch (ParserConfigurationException ex) { + throw new HttpMessageNotReadableException("Could not set feature: " + ex.getMessage(), ex); + } + catch (SAXException ex) { + throw new HttpMessageNotReadableException("Could not parse document: " + ex.getMessage(), ex); } } - private ByteArrayInputStream transformToByteArrayInputStream(Source source) throws TransformerException { - ByteArrayOutputStream bos = new ByteArrayOutputStream(); - transform(source, new StreamResult(bos)); - return new ByteArrayInputStream(bos.toByteArray()); + private SAXSource readSAXSource(InputStream body) throws IOException { + try { + XMLReader reader = XMLReaderFactory.createXMLReader(); + reader.setFeature("http://xml.org/sax/features/external-general-entities", processExternalEntities); + byte[] bytes = StreamUtils.copyToByteArray(body); + return new SAXSource(reader, new InputSource(new ByteArrayInputStream(bytes))); + } + catch (SAXException ex) { + throw new HttpMessageNotReadableException("Could not parse document: " + ex.getMessage(), ex); + } + } + + private Source readStAXSource(InputStream body) { + try { + XMLInputFactory inputFactory = XMLInputFactory.newFactory(); + inputFactory.setProperty("javax.xml.stream.isSupportingExternalEntities", processExternalEntities); + XMLStreamReader streamReader = inputFactory.createXMLStreamReader(body); + return new StAXSource(streamReader); + } + catch (XMLStreamException ex) { + throw new HttpMessageNotReadableException("Could not parse document: " + ex.getMessage(), ex); + } + } + + private StreamSource readStreamSource(InputStream body) throws IOException { + byte[] bytes = StreamUtils.copyToByteArray(body); + return new StreamSource(new ByteArrayInputStream(bytes)); } @Override @@ -102,15 +173,22 @@ protected Long getContentLength(T t, MediaType contentType) { } @Override - protected void writeToResult(T t, HttpHeaders headers, Result result) throws IOException { + protected void writeInternal(T t, HttpOutputMessage outputMessage) + throws IOException, HttpMessageNotWritableException { try { + Result result = new StreamResult(outputMessage.getBody()); transform(t, result); } catch (TransformerException ex) { - throw new HttpMessageNotWritableException("Could not transform [" + t + "] to [" + result + "]", ex); + throw new HttpMessageNotWritableException("Could not transform [" + t + "] to output message", ex); } } + private void transform(Source source, Result result) throws TransformerException { + this.transformerFactory.newTransformer().transform(source, result); + } + + private static class CountingOutputStream extends OutputStream { private long count = 0; spring-web/src/test/java/org/springframework/http/converter/xml/SourceHttpMessageConverterTests.java View file @ 2ae6a6a @@ -17,21 +17,29 @@ package org.springframework.http.converter.xml; import static org.custommonkey.xmlunit.XMLAssert.assertXMLEqual; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertTrue; +import static org.junit.Assert.*; +import static org.junit.Assert.assertNotEquals; +import java.io.IOException; +import java.io.InputStream; import java.io.InputStreamReader; import java.io.StringReader; import java.nio.charset.Charset; import javax.xml.parsers.DocumentBuilderFactory; +import javax.xml.stream.XMLStreamException; +import javax.xml.stream.XMLStreamReader; import javax.xml.transform.Source; import javax.xml.transform.dom.DOMSource; import javax.xml.transform.sax.SAXSource; +import javax.xml.transform.stax.StAXSource; import javax.xml.transform.stream.StreamSource; import org.junit.Before; import org.junit.Test; + +import org.springframework.core.io.ClassPathResource; +import org.springframework.core.io.Resource; import org.springframework.http.MediaType; import org.springframework.http.MockHttpInputMessage; import org.springframework.http.MockHttpOutputMessage; @@ -39,17 +47,29 @@ import org.w3c.dom.Document; import org.w3c.dom.Element; import org.xml.sax.InputSource; +import org.xml.sax.SAXException; +import org.xml.sax.XMLReader; +import org.xml.sax.helpers.DefaultHandler; /** * @author Arjen Poutsma */ public class SourceHttpMessageConverterTests { + private static final String BODY = "<root>Hello World</root>"; + private SourceHttpMessageConverter<Source> converter; + private String bodyExternal; + @Before - public void setUp() { + public void setUp() throws IOException { converter = new SourceHttpMessageConverter<Source>(); + Resource external = new ClassPathResource("external.txt", getClass()); + + bodyExternal = "<!DOCTYPE root [" + + " <!ELEMENT root ANY >\n" + + " <!ENTITY ext SYSTEM \"" + external.getURI() + "\" >]><root>&ext;</root>"; } @Test @@ -67,8 +87,7 @@ public void canWrite() { @Test public void readDOMSource() throws Exception { - String body = "<root>Hello World</root>"; - MockHttpInputMessage inputMessage = new MockHttpInputMessage(body.getBytes("UTF-8")); + MockHttpInputMessage inputMessage = new MockHttpInputMessage(BODY.getBytes("UTF-8")); inputMessage.getHeaders().setContentType(new MediaType("application", "xml")); DOMSource result = (DOMSource) converter.read(DOMSource.class, inputMessage); Document document = (Document) result.getNode(); @@ -76,30 +95,86 @@ public void readDOMSource() throws Exception { } @Test + public void readDOMSourceExternal() throws Exception { + MockHttpInputMessage inputMessage = new MockHttpInputMessage(bodyExternal.getBytes("UTF-8")); + inputMessage.getHeaders().setContentType(new MediaType("application", "xml")); + 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 readSAXSource() throws Exception { - String body = "<root>Hello World</root>"; - MockHttpInputMessage inputMessage = new MockHttpInputMessage(body.getBytes("UTF-8")); + MockHttpInputMessage inputMessage = new MockHttpInputMessage(BODY.getBytes("UTF-8")); inputMessage.getHeaders().setContentType(new MediaType("application", "xml")); SAXSource result = (SAXSource) converter.read(SAXSource.class, inputMessage); InputSource inputSource = result.getInputSource(); String s = FileCopyUtils.copyToString(new InputStreamReader(inputSource.getByteStream())); - assertXMLEqual("Invalid result", body, s); + assertXMLEqual("Invalid result", BODY, s); + } + + @Test + public void readSAXSourceExternal() throws Exception { + MockHttpInputMessage inputMessage = new MockHttpInputMessage(bodyExternal.getBytes("UTF-8")); + inputMessage.getHeaders().setContentType(new MediaType("application", "xml")); + SAXSource result = (SAXSource) converter.read(SAXSource.class, inputMessage); + InputSource inputSource = result.getInputSource(); + XMLReader reader = result.getXMLReader(); + reader.setContentHandler(new DefaultHandler() { + @Override + public void characters(char[] ch, int start, int length) throws SAXException { + String s = new String(ch, start, length); + assertNotEquals("Invalid result", "Foo Bar", s); + } + }); + reader.parse(inputSource); } @Test + public void readStAXSource() throws Exception { + MockHttpInputMessage inputMessage = new MockHttpInputMessage(BODY.getBytes("UTF-8")); + inputMessage.getHeaders().setContentType(new MediaType("application", "xml")); + StAXSource result = (StAXSource) converter.read(StAXSource.class, inputMessage); + XMLStreamReader streamReader = result.getXMLStreamReader(); + assertTrue(streamReader.hasNext()); + streamReader.nextTag(); + String s = streamReader.getLocalName(); + assertEquals("root", s); + s = streamReader.getElementText(); + assertEquals("Hello World", s); + streamReader.close(); + } + + @Test + public void readStAXSourceExternal() throws Exception { + MockHttpInputMessage inputMessage = new MockHttpInputMessage(bodyExternal.getBytes("UTF-8")); + inputMessage.getHeaders().setContentType(new MediaType("application", "xml")); + StAXSource result = (StAXSource) converter.read(StAXSource.class, inputMessage); + XMLStreamReader streamReader = result.getXMLStreamReader(); + assertTrue(streamReader.hasNext()); + streamReader.next(); + streamReader.next(); + String s = streamReader.getLocalName(); + assertEquals("root", s); + s = streamReader.getElementText(); + assertNotEquals("Foo Bar", s); + streamReader.close(); + } + + + @Test public void readStreamSource() throws Exception { - String body = "<root>Hello World</root>"; - MockHttpInputMessage inputMessage = new MockHttpInputMessage(body.getBytes("UTF-8")); + MockHttpInputMessage inputMessage = new MockHttpInputMessage(BODY.getBytes("UTF-8")); inputMessage.getHeaders().setContentType(new MediaType("application", "xml")); StreamSource result = (StreamSource) converter.read(StreamSource.class, inputMessage); String s = FileCopyUtils.copyToString(new InputStreamReader(result.getInputStream())); - assertXMLEqual("Invalid result", body, s); + assertXMLEqual("Invalid result", BODY, s); } @Test public void readSource() throws Exception { - String body = "<root>Hello World</root>"; - MockHttpInputMessage inputMessage = new MockHttpInputMessage(body.getBytes("UTF-8")); + MockHttpInputMessage inputMessage = new MockHttpInputMessage(BODY.getBytes("UTF-8")); inputMessage.getHeaders().setContentType(new MediaType("application", "xml")); converter.read(Source.class, inputMessage); }