From dd3be31f7062dcb2f3b2d7f0e89df29e197dee63 Mon Sep 17 00:00:00 2001
From: Uwe Schindler <uschindler@apache.org>
Date: Sun, 18 Feb 2018 22:41:06 +0100
Subject: [PATCH] SOLR-11971: Don't allow referal to external resources in
 DataImportHandler's dataConfig request parameter

---
 solr/CHANGES.txt                              |  6 ++++--
 .../solr/handler/dataimport/DataImporter.java | 17 ++++++++++++----
 .../handler/dataimport/TestErrorHandling.java | 20 +++++++++++++++++++
 3 files changed, 37 insertions(+), 6 deletions(-)

diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 6cef677ca1df..1dda7a30dbf9 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -29,9 +29,11 @@ Apache UIMA 2.3.1
 Apache ZooKeeper 3.4.10
 Jetty 9.3.14.v20161028
 
+Bug Fixes
+----------------------
 
-(No Changes)
-
+* SOLR-11971: Don't allow referal to external resources in DataImportHandler's dataConfig request parameter.
+  (麦 香浓郁, Uwe Schindler)
 
 ==================  6.6.2 ==================
 
diff --git a/solr/contrib/dataimporthandler/src/java/org/apache/solr/handler/dataimport/DataImporter.java b/solr/contrib/dataimporthandler/src/java/org/apache/solr/handler/dataimport/DataImporter.java
index a49b4f6285bf..4825fd1225b9 100644
--- a/solr/contrib/dataimporthandler/src/java/org/apache/solr/handler/dataimport/DataImporter.java
+++ b/solr/contrib/dataimporthandler/src/java/org/apache/solr/handler/dataimport/DataImporter.java
@@ -16,6 +16,7 @@
  */
 package org.apache.solr.handler.dataimport;
 
+import org.apache.solr.common.EmptyEntityResolver;
 import org.apache.solr.common.SolrException;
 import org.apache.solr.core.SolrCore;
 import org.apache.solr.schema.IndexSchema;
@@ -178,11 +179,11 @@ public IndexSchema getSchema() {
   /**
    * Used by tests
    */
-  public void loadAndInit(String configStr) {
+  void loadAndInit(String configStr) {
     config = loadDataConfig(new InputSource(new StringReader(configStr)));
   }
 
-  public void loadAndInit(InputSource configFile) {
+  void loadAndInit(InputSource configFile) {
     config = loadDataConfig(configFile);
   }
 
@@ -191,8 +192,10 @@ public DIHConfiguration loadDataConfig(InputSource configFile) {
     DIHConfiguration dihcfg = null;
     try {
       DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance();
+      dbf.setValidating(false);
       
-      // only enable xinclude, if a a SolrCore and SystemId is present (makes no sense otherwise)
+      // only enable xinclude, if XML is coming from safe source (local file)
+      // and a a SolrCore and SystemId is present (makes no sense otherwise):
       if (core != null && configFile.getSystemId() != null) {
         try {
           dbf.setXIncludeAware(true);
@@ -203,8 +206,14 @@ public DIHConfiguration loadDataConfig(InputSource configFile) {
       }
       
       DocumentBuilder builder = dbf.newDocumentBuilder();
-      if (core != null)
+      // only enable xinclude / external entities, if XML is coming from
+      // safe source (local file) and a a SolrCore and SystemId is present:
+      if (core != null && configFile.getSystemId() != null) {
         builder.setEntityResolver(new SystemIdResolver(core.getResourceLoader()));
+      } else {
+        // Don't allow external entities without having a system ID:
+        builder.setEntityResolver(EmptyEntityResolver.SAX_INSTANCE);
+      }
       builder.setErrorHandler(XMLLOG);
       Document document;
       try {
diff --git a/solr/contrib/dataimporthandler/src/test/org/apache/solr/handler/dataimport/TestErrorHandling.java b/solr/contrib/dataimporthandler/src/test/org/apache/solr/handler/dataimport/TestErrorHandling.java
index 74eaf9e20f65..1ea1ad483a1d 100644
--- a/solr/contrib/dataimporthandler/src/test/org/apache/solr/handler/dataimport/TestErrorHandling.java
+++ b/solr/contrib/dataimporthandler/src/test/org/apache/solr/handler/dataimport/TestErrorHandling.java
@@ -89,6 +89,13 @@ public void testTransformerErrorContinue() throws Exception {
     assertQ(req("*:*"), "//*[@numFound='3']");
   }
 
+  public void testExternalEntity() throws Exception {
+    StringDataSource.xml = wellformedXml;
+    // This should not fail as external entities are replaced by an empty string during parsing:
+    runFullImport(dataConfigWithEntity);
+    assertQ(req("*:*"), "//*[@numFound='3']");
+  }
+
   public static class StringDataSource extends DataSource<Reader> {
     public static String xml = "";
 
@@ -157,6 +164,19 @@ public Object transformRow(Map<String, Object> row, Context context) {
           "    </document>\n" +
           "</dataConfig>";
 
+  private String dataConfigWithEntity = "<!DOCTYPE dataConfig [\n" + 
+          "  <!ENTITY internalTerm \"node\">\n" + 
+          "  <!ENTITY externalTerm SYSTEM \"foo://bar.xyz/external\">\n" + 
+          "]><dataConfig>\n" +
+          "    <dataSource name=\"str\" type=\"TestErrorHandling$StringDataSource\" />" +
+          "    <document>\n" +
+          "        <entity name=\"&internalTerm;\" dataSource=\"str\" processor=\"XPathEntityProcessor\" url=\"test\" forEach=\"/root/node\" onError=\"skip\">\n" +
+          "            <field column=\"id\" xpath=\"/root/node/id\">&externalTerm;</field>\n" +
+          "            <field column=\"desc\" xpath=\"/root/node/desc\" />\n" +
+          "        </entity>\n" +
+          "    </document>\n" +
+          "</dataConfig>";
+
   private String malformedXml = "<root>\n" +
           "    <node>\n" +
           "        <id>1</id>\n" +