Permalink
Browse files

Add processExternalEntities support to OXM

Update OXM AbstractMarshaller to support processing of external
XML entities. By default external entities will not be processed.

Issue: SPR-11376
  • Loading branch information...
1 parent 7efd54e commit edba32b3093703d5e9ed42b5b8ec23ecc1998398 @rstoyanchev rstoyanchev committed with philwebb Feb 18, 2014
@@ -162,6 +162,11 @@ public void setEncoding(String encoding) {
this.encoding = encoding;
}
+ @Override
+ protected String getDefaultEncoding() {
+ return this.encoding;
+ }
+
/**
* Set the locations of the Castor XML mapping files.
*/
@@ -400,6 +400,13 @@ public void setProcessExternalEntities(boolean processExternalEntities) {
this.processExternalEntities = processExternalEntities;
}
+ /**
+ * @return the configured value for whether XML external entities are allowed.
+ */
+ public boolean isProcessExternalEntities() {
+ return this.processExternalEntities;
+ }
+
@Override
public void setBeanClassLoader(ClassLoader classLoader) {
this.beanClassLoader = classLoader;
@@ -1,5 +1,5 @@
/*
- * Copyright 2002-2013 the original author or authors.
+ * Copyright 2002-2014 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,6 +28,7 @@
import javax.xml.stream.XMLStreamException;
import javax.xml.stream.XMLStreamReader;
import javax.xml.stream.XMLStreamWriter;
+import javax.xml.transform.OutputKeys;
import javax.xml.transform.Result;
import javax.xml.transform.Source;
import javax.xml.transform.Transformer;
@@ -149,6 +150,11 @@ public void setEncoding(String encoding) {
this.encoding = encoding;
}
+ @Override
+ protected String getDefaultEncoding() {
+ return this.encoding;
+ }
+
/**
* Set the document standalone flag for marshalling. By default, this flag is not present.
*/
@@ -338,7 +344,7 @@ private void transformAndMarshal(Object graph, Result result) throws IOException
}
catch (TransformerException ex) {
throw new MarshallingFailureException(
- "Could not transform to [" + ClassUtils.getShortName(result.getClass()) + "]");
+ "Could not transform to [" + ClassUtils.getShortName(result.getClass()) + "]", ex);
}
}
@@ -398,7 +404,7 @@ protected Object unmarshalReader(Reader reader) throws XmlMappingException, IOEx
@Override
protected Object unmarshalDomNode(Node node) throws XmlMappingException {
try {
- return transformAndUnmarshal(new DOMSource(node));
+ return transformAndUnmarshal(new DOMSource(node), null);
}
catch (IOException ex) {
throw new UnmarshallingFailureException("JiBX unmarshalling exception", ex);
@@ -409,20 +415,23 @@ protected Object unmarshalDomNode(Node node) throws XmlMappingException {
protected Object unmarshalSaxReader(XMLReader xmlReader, InputSource inputSource)
throws XmlMappingException, IOException {
- return transformAndUnmarshal(new SAXSource(xmlReader, inputSource));
+ return transformAndUnmarshal(new SAXSource(xmlReader, inputSource), inputSource.getEncoding());
}
- private Object transformAndUnmarshal(Source source) throws IOException {
+ private Object transformAndUnmarshal(Source source, String encoding) throws IOException {
try {
Transformer transformer = this.transformerFactory.newTransformer();
+ if (encoding != null) {
+ transformer.setOutputProperty(OutputKeys.ENCODING, encoding);
+ }
ByteArrayOutputStream os = new ByteArrayOutputStream();
transformer.transform(source, new StreamResult(os));
ByteArrayInputStream is = new ByteArrayInputStream(os.toByteArray());
return unmarshalInputStream(is);
}
catch (TransformerException ex) {
throw new MarshallingFailureException(
- "Could not transform from [" + ClassUtils.getShortName(source.getClass()) + "]");
+ "Could not transform from [" + ClassUtils.getShortName(source.getClass()) + "]", ex);
}
}
@@ -1,5 +1,5 @@
/*
- * Copyright 2002-2013 the original author or authors.
+ * Copyright 2002-2014 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.
@@ -73,6 +73,34 @@
private final Object documentBuilderFactoryMonitor = new Object();
+ private boolean processExternalEntities = false;
+
+
+ /**
+ * Indicates whether external XML entities are processed when unmarshalling.
+ * <p>Default is {@code false}, meaning that external entities are not resolved.
+ * Note that processing of external entities will only be enabled/disabled when the
+ * {@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.
+ */
+ public void setProcessExternalEntities(boolean processExternalEntities) {
+ this.processExternalEntities = processExternalEntities;
+ }
+
+ /**
+ * @return the configured value for whether XML external entities are allowed.
+ */
+ public boolean isProcessExternalEntities() {
+ return this.processExternalEntities;
+ }
+
+ /**
+ * @return the default encoding to use for marshalling or unmarshalling from
+ * a byte stream, or {@code null}.
+ */
+ abstract protected String getDefaultEncoding();
+
/**
* Marshals the object graph with the given root into the provided {@code javax.xml.transform.Result}.
@@ -133,7 +161,7 @@ else if (source instanceof SAXSource) {
return unmarshalSaxSource((SAXSource) source);
}
else if (source instanceof StreamSource) {
- return unmarshalStreamSource((StreamSource) source);
+ return unmarshalStreamSourceNoExternalEntitities((StreamSource) source);
}
else {
throw new IllegalArgumentException("Unknown Source type: " + source.getClass());
@@ -175,7 +203,9 @@ protected DocumentBuilderFactory createDocumentBuilderFactory() throws ParserCon
* @throws SAXException if thrown by JAXP methods
*/
protected XMLReader createXmlReader() throws SAXException {
- return XMLReaderFactory.createXMLReader();
+ XMLReader xmlReader = XMLReaderFactory.createXMLReader();
+ xmlReader.setFeature("http://xml.org/sax/features/external-general-entities", isProcessExternalEntities());
+ return xmlReader;
}
@@ -358,8 +388,42 @@ protected Object unmarshalSaxSource(SAXSource saxSource) throws XmlMappingExcept
}
/**
+ * Template method for handling {@code StreamSource}s with protection against
+ * the XML External Entity (XXE) processing vulnerability taking into account
+ * the value of the {@link #setProcessExternalEntities(boolean)} property.
+ * <p>
+ * The default implementation wraps the StreamSource as a SAXSource and delegates
+ * to {@link #unmarshalSaxSource(javax.xml.transform.sax.SAXSource)}.
+ *
+ * @param streamSource the {@code StreamSource}
+ * @return the object graph
+ * @throws IOException if an I/O exception occurs
+ * @throws XmlMappingException if the given source cannot be mapped to an object
+ *
+ * @see <a href="https://www.owasp.org/index.php/XML_External_Entity_(XXE)_Processing">XML_External_Entity_(XXE)_Processing</a>
+ */
+ protected Object unmarshalStreamSourceNoExternalEntitities(StreamSource streamSource) throws XmlMappingException, IOException {
+ InputSource inputSource;
+ if (streamSource.getInputStream() != null) {
+ inputSource = new InputSource(streamSource.getInputStream());
+ inputSource.setEncoding(getDefaultEncoding());
+ }
+ else if (streamSource.getReader() != null) {
+ inputSource = new InputSource(streamSource.getReader());
+ }
+ else {
+ inputSource = new InputSource(streamSource.getSystemId());
+ }
+ return unmarshalSaxSource(new SAXSource(inputSource));
+ }
+
+ /**
* Template method for handling {@code StreamSource}s.
* <p>This implementation defers to {@code unmarshalInputStream} or {@code unmarshalReader}.
+ * <p>As of 3.2.8 and 4.0.2 this method is no longer invoked from
+ * {@link #unmarshal(javax.xml.transform.Source)}. The method invoked instead is
+ * {@link #unmarshalStreamSourceNoExternalEntitities(javax.xml.transform.stream.StreamSource)}.
+ *
* @param streamSource the {@code StreamSource}
* @return the object graph
* @throws IOException if an I/O exception occurs
@@ -1,5 +1,5 @@
/*
- * Copyright 2002-2012 the original author or authors.
+ * Copyright 2002-2014 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.
@@ -113,6 +113,10 @@ public boolean isValidating() {
return this.validating;
}
+ @Override
+ protected String getDefaultEncoding() {
+ return null;
+ }
/**
* This implementation returns true if the given class is an implementation of {@link XmlObject}.
@@ -27,11 +27,9 @@
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
-import javax.xml.stream.XMLEventReader;
-import javax.xml.stream.XMLEventWriter;
-import javax.xml.stream.XMLStreamException;
-import javax.xml.stream.XMLStreamReader;
-import javax.xml.stream.XMLStreamWriter;
+import javax.xml.stream.*;
+import javax.xml.transform.stax.StAXSource;
+import javax.xml.transform.stream.StreamSource;
import com.thoughtworks.xstream.MarshallingStrategy;
import com.thoughtworks.xstream.XStream;
@@ -342,6 +340,11 @@ public void setEncoding(String encoding) {
this.encoding = encoding;
}
+ @Override
+ protected String getDefaultEncoding() {
+ return this.encoding;
+ }
+
/**
* Set the classes supported by this marshaller.
* <p>If this property is empty (the default), all classes are supported.
@@ -701,6 +704,13 @@ private void doMarshal(Object graph, HierarchicalStreamWriter streamWriter, Data
// Unmarshalling
@Override
+ protected Object unmarshalStreamSourceNoExternalEntitities(StreamSource streamSource)
+ throws XmlMappingException, IOException {
+
+ return super.unmarshalStreamSource(streamSource);
+ }
+
+ @Override
protected Object unmarshalDomNode(Node node) throws XmlMappingException {
HierarchicalStreamReader streamReader;
if (node instanceof Document) {
@@ -1,5 +1,5 @@
/*
- * Copyright 2002-2013 the original author or authors.
+ * Copyright 2002-2014 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.
@@ -19,6 +19,8 @@
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;
import org.junit.Ignore;
@@ -28,9 +30,13 @@
import org.springframework.oxm.AbstractUnmarshallerTests;
import org.springframework.oxm.MarshallingException;
import org.springframework.oxm.Unmarshaller;
+import org.xml.sax.InputSource;
+import org.xml.sax.XMLReader;
+import static junit.framework.Assert.assertNotNull;
import static org.hamcrest.CoreMatchers.*;
import static org.junit.Assert.*;
+import static org.junit.Assert.assertEquals;
/**
* @author Arjen Poutsma
@@ -203,4 +209,59 @@ private Object unmarshal(String xml) throws Exception {
StreamSource source = new StreamSource(new StringReader(xml));
return unmarshaller.unmarshal(source);
}
+
+ @Test
+ public void unmarshalStreamSourceExternalEntities() throws Exception {
+
+ final AtomicReference<XMLReader> result = new AtomicReference<XMLReader>();
+ CastorMarshaller marshaller = new CastorMarshaller() {
+ @Override
+ protected Object unmarshalSaxReader(XMLReader xmlReader, InputSource inputSource) {
+ result.set(xmlReader);
+ return null;
+ }
+ };
+
+ // 1. external-general-entities disabled (default)
+
+ marshaller.unmarshal(new StreamSource("1"));
+ assertNotNull(result.get());
+ assertEquals(false, result.get().getFeature("http://xml.org/sax/features/external-general-entities"));
+
+ // 2. external-general-entities disabled (default)
+
+ result.set(null);
+ marshaller.setProcessExternalEntities(true);
+ marshaller.unmarshal(new StreamSource("1"));
+ assertNotNull(result.get());
+ assertEquals(true, result.get().getFeature("http://xml.org/sax/features/external-general-entities"));
+ }
+
+ @Test
+ public void unmarshalSaxSourceExternalEntities() throws Exception {
+
+ final AtomicReference<XMLReader> result = new AtomicReference<XMLReader>();
+ CastorMarshaller marshaller = new CastorMarshaller() {
+ @Override
+ protected Object unmarshalSaxReader(XMLReader xmlReader, InputSource inputSource) {
+ result.set(xmlReader);
+ return null;
+ }
+ };
+
+ // 1. external-general-entities disabled (default)
+
+ marshaller.unmarshal(new SAXSource(new InputSource("1")));
+ assertNotNull(result.get());
+ assertEquals(false, result.get().getFeature("http://xml.org/sax/features/external-general-entities"));
+
+ // 2. external-general-entities disabled (default)
+
+ result.set(null);
+ marshaller.setProcessExternalEntities(true);
+ marshaller.unmarshal(new SAXSource(new InputSource("1")));
+ assertNotNull(result.get());
+ assertEquals(true, result.get().getFeature("http://xml.org/sax/features/external-general-entities"));
+ }
+
}
Oops, something went wrong.

0 comments on commit edba32b

Please sign in to comment.