Skip to content

Commit

Permalink
SOLR-11971: Don't allow referal to external resources in DataImportHa…
Browse files Browse the repository at this point in the history
…ndler's dataConfig request parameter
  • Loading branch information
uschindler committed Mar 1, 2018
1 parent 51e712c commit dd3be31
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 6 deletions.
6 changes: 4 additions & 2 deletions solr/CHANGES.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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 ==================

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}

Expand All @@ -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);
Expand All @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 = "";

Expand Down Expand Up @@ -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" +
Expand Down

0 comments on commit dd3be31

Please # to comment.