Merge pull request #2735 from square/jwilson.0417.lock_down_jaxb

Lock down JAXB. Don't load remote entities.
square · Apr 17, 2018 · 97057aa · 97057aa
2 parents ce787ed + 4a693c5
commit 97057aa
Show file tree

Hide file tree

Showing 2 changed files with 55 additions and 0 deletions.
diff --git a/retrofit-converters/jaxb/src/main/java/retrofit2/converter/jaxb/JaxbResponseConverter.java b/retrofit-converters/jaxb/src/main/java/retrofit2/converter/jaxb/JaxbResponseConverter.java
@@ -33,6 +33,10 @@ final class JaxbResponseConverter<T> implements Converter<ResponseBody, T> {
   JaxbResponseConverter(JAXBContext context, Class<T> type) {
     this.context = context;
     this.type = type;
+
+    // Prevent XML External Entity attacks (XXE).
+    xmlInputFactory.setProperty(XMLInputFactory.IS_SUPPORTING_EXTERNAL_ENTITIES, false);
+    xmlInputFactory.setProperty(XMLInputFactory.SUPPORT_DTD, false);
   }
 
   @Override public T convert(ResponseBody value) throws IOException {

diff --git a/...ofit-converters/jaxb/src/test/java/retrofit2/converter/jaxb/JaxbConverterFactoryTest.java b/...ofit-converters/jaxb/src/test/java/retrofit2/converter/jaxb/JaxbConverterFactoryTest.java
@@ -146,4 +146,55 @@ interface Service {
     Response<Contact> response = call.execute();
     assertThat(response.body().name).isEqualTo("Jenny");
   }
+
+  @Test public void externalEntity() throws Exception {
+    server.enqueue(new MockResponse()
+        .setBody(""
+            + "<?xml version=\"1.0\" ?>"
+            + "<!DOCTYPE contact["
+            + "  <!ENTITY secret SYSTEM \"" + server.url("/secret.txt") + "\">"
+            + "]>"
+            + "<contact>"
+            + "<name>&secret;</name>"
+            + "</contact>"));
+    server.enqueue(new MockResponse()
+        .setBody("hello"));
+
+    Call<Contact> call = service.getXml();
+    try {
+      Response<Contact> response = call.execute();
+      response.body();
+      fail();
+    } catch (RuntimeException expected) {
+      assertThat(expected).hasMessageContaining("ParseError");
+    }
+
+    assertThat(server.getRequestCount()).isEqualTo(1);
+  }
+
+  @Test public void externalDtd() throws Exception {
+    server.enqueue(new MockResponse()
+        .setBody(""
+            + "<?xml version=\"1.0\" ?>"
+            + "<!DOCTYPE contact SYSTEM \"" + server.url("/contact.dtd") + "\">"
+            + "<contact>"
+            + "<name>&secret;</name>"
+            + "</contact>"));
+    server.enqueue(new MockResponse()
+        .setBody(""
+            + "<!ELEMENT contact (name)>\n"
+            + "<!ELEMENT name (#PCDATA)>\n"
+            + "<!ENTITY secret \"hello\">"));
+
+    Call<Contact> call = service.getXml();
+    try {
+      Response<Contact> response = call.execute();
+      response.body();
+      fail();
+    } catch (RuntimeException expected) {
+      assertThat(expected).hasMessageContaining("ParseError");
+    }
+
+    assertThat(server.getRequestCount()).isEqualTo(1);
+  }
 }