From dbb044cfa0f2db45e6b6065c940a247e2eef2a3a Mon Sep 17 00:00:00 2001
From: jckautzmann <jkautzma@adobe.com>
Date: Fri, 21 Jan 2022 12:33:02 +0100
Subject: [PATCH] GH-1936 - [Embed] XXE in OEmbedClientImpl.java (#1938)

- use a secured parser to parse the response before unmarshalling it
---
 .../services/embed/OEmbedClientImpl.java      | 23 +++++++++++++++----
 .../services/embed/OEmbedClientImplTest.java  |  3 ++-
 2 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/bundles/core/src/main/java/com/adobe/cq/wcm/core/components/internal/services/embed/OEmbedClientImpl.java b/bundles/core/src/main/java/com/adobe/cq/wcm/core/components/internal/services/embed/OEmbedClientImpl.java
index 15e0ecc4e2..798ae619b1 100644
--- a/bundles/core/src/main/java/com/adobe/cq/wcm/core/components/internal/services/embed/OEmbedClientImpl.java
+++ b/bundles/core/src/main/java/com/adobe/cq/wcm/core/components/internal/services/embed/OEmbedClientImpl.java
@@ -25,13 +25,16 @@
 import javax.xml.bind.JAXBContext;
 import javax.xml.bind.JAXBException;
 import javax.xml.bind.Unmarshaller;
+import javax.xml.parsers.ParserConfigurationException;
+import javax.xml.parsers.SAXParserFactory;
+import javax.xml.transform.Source;
+import javax.xml.transform.sax.SAXSource;
 
 import org.apache.commons.lang3.StringUtils;
 import org.apache.http.HttpResponse;
 import org.apache.http.client.HttpClient;
 import org.apache.http.client.config.RequestConfig;
 import org.apache.http.client.methods.HttpGet;
-import org.apache.http.config.SocketConfig;
 import org.apache.http.impl.client.HttpClients;
 import org.apache.http.osgi.services.HttpClientBuilderFactory;
 import org.osgi.framework.Constants;
@@ -41,6 +44,8 @@
 import org.osgi.service.component.annotations.ReferencePolicy;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
+import org.xml.sax.InputSource;
+import org.xml.sax.SAXException;
 
 import com.adobe.cq.wcm.core.components.services.embed.OEmbedClient;
 import com.adobe.cq.wcm.core.components.services.embed.OEmbedResponse;
@@ -106,9 +111,19 @@ public OEmbedResponse getResponse(String url) {
         } else if (jaxbContext != null && OEmbedResponse.Format.XML == OEmbedResponse.Format.fromString(config.format())) {
             try {
                 String xmlURL = buildURL(config.endpoint(), url, OEmbedResponse.Format.XML.getValue(), null, null);
-                Unmarshaller jaxbUnmarshaller = jaxbContext.createUnmarshaller();
-                return (OEmbedResponse) jaxbUnmarshaller.unmarshal(getData(xmlURL));
-            } catch (JAXBException | IOException e) {
+                try (InputStream xmlStream = getData(xmlURL)) {
+                    //Disable XXE
+                    SAXParserFactory spf = SAXParserFactory.newInstance();
+                    spf.setFeature("http://xml.org/sax/features/external-general-entities", false);
+                    spf.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
+                    spf.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
+
+                    //Do unmarshall operation
+                    Source xmlSource = new SAXSource(spf.newSAXParser().getXMLReader(), new InputSource(xmlStream));
+                    Unmarshaller jaxbUnmarshaller = jaxbContext.createUnmarshaller();
+                    return (OEmbedResponse) jaxbUnmarshaller.unmarshal(xmlSource);
+                }
+            } catch (SAXException | ParserConfigurationException | JAXBException | IOException e) {
                 LOGGER.error("Failed to read JSON response", e);
             }
         }
diff --git a/bundles/core/src/test/java/com/adobe/cq/wcm/core/components/internal/services/embed/OEmbedClientImplTest.java b/bundles/core/src/test/java/com/adobe/cq/wcm/core/components/internal/services/embed/OEmbedClientImplTest.java
index 448c1d178a..db0b0fb24f 100644
--- a/bundles/core/src/test/java/com/adobe/cq/wcm/core/components/internal/services/embed/OEmbedClientImplTest.java
+++ b/bundles/core/src/test/java/com/adobe/cq/wcm/core/components/internal/services/embed/OEmbedClientImplTest.java
@@ -23,6 +23,7 @@
 import javax.xml.bind.JAXBContext;
 import javax.xml.bind.JAXBException;
 import javax.xml.bind.Unmarshaller;
+import javax.xml.transform.Source;
 
 import org.apache.http.HttpEntity;
 import org.apache.http.client.config.RequestConfig;
@@ -145,7 +146,7 @@ public boolean unsafeContext() {
         Unmarshaller unmarshaller = mock(Unmarshaller.class);
         when(jaxbContext.createUnmarshaller()).thenReturn(unmarshaller);
         mockHttpClient(client);
-        when(unmarshaller.unmarshal(any(InputStream.class))).thenReturn(new OEmbedXMLResponseImpl());
+        when(unmarshaller.unmarshal(any(Source.class))).thenReturn(new OEmbedXMLResponseImpl());
         String provider = client.getProvider("http://test.com/xml");
         assertEquals("Test XML", provider);
         assertNotNull(client.getResponse("http://test.com/xml"));