SOLR-11477: Disallow resolving of external entities in Lucene queryparser/xml/CoreParser and SolrCoreParser (defType=xmlparser or {!xmlparser ...}) by default.

(Michael Stepankin, Olga Barinova, Uwe Schindler, Christine Poerschke)

Author: cpoerschke

lucene/CHANGES.txt

@@ -5,6 +5,11 @@ http://s.apache.org/luceneversions
 
 ======================= Lucene 7.1.0 =======================
 
+Changes in Runtime Behavior
+
+* Resolving of external entities in queryparser/xml/CoreParser is disallowed
+  by default. See SOLR-11477 for details.
+
 New Features
 
 * LUCENE-7970: Add a shape to Geo3D that consists of multiple planes that
@@ -111,6 +116,9 @@ Bug Fixes
 * LUCENE-7957: ConjunctionScorer.getChildren was failing to return all
   child scorers (Adrien Grand, Mike McCandless)
 
+* SOLR-11477: Disallow resolving of external entities in queryparser/xml/CoreParser
+  by default. (Michael Stepankin, Olga Barinova, Uwe Schindler, Christine Poerschke)
+
 Build
 
 * SOLR-11181: Switch order of maven artifact publishing procedure: deploy first

lucene/queryparser/src/java/org/apache/lucene/queryparser/xml/CoreParser.java

@@ -23,11 +23,17 @@
 import org.apache.lucene.search.spans.SpanQuery;
 import org.w3c.dom.Document;
 import org.w3c.dom.Element;
+import org.xml.sax.EntityResolver;
+import org.xml.sax.ErrorHandler;
+import org.xml.sax.SAXException;
 
+import javax.xml.XMLConstants;
 import javax.xml.parsers.DocumentBuilder;
 import javax.xml.parsers.DocumentBuilderFactory;
+import javax.xml.parsers.ParserConfigurationException;
 
 import java.io.InputStream;
+import java.util.Locale;
 
 /**
  * Assembles a QueryBuilder which uses only core Lucene Query objects
@@ -111,6 +117,10 @@ protected CoreParser(String defaultField, Analyzer analyzer, QueryParser parser)
     queryFactory.addBuilder("SpanNot", snot);
   }
 
+  /**
+   * Parses the given stream as XML file and returns a {@link Query}.
+   * By default this disallows external entities for security reasons.
+   */
   public Query parse(InputStream xmlStream) throws ParserException {
     return getQuery(parseXML(xmlStream).getDocumentElement());
   }
@@ -133,23 +143,47 @@ public void addSpanQueryBuilder(String nodeName, SpanQueryBuilder builder) {
     spanFactory.addBuilder(nodeName, builder);
   }
 
-  static Document parseXML(InputStream pXmlFile) throws ParserException {
-    DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance();
-    DocumentBuilder db = null;
+  /**
+   * Returns a SAX {@link EntityResolver} to be used by {@link DocumentBuilder}.
+   * By default this returns {@link #DISALLOW_EXTERNAL_ENTITY_RESOLVER}, which disallows the
+   * expansion of external entities (for security reasons). To restore legacy behavior,
+   * override this method to return {@code null}.
+   */
+  protected EntityResolver getEntityResolver() {
+    return DISALLOW_EXTERNAL_ENTITY_RESOLVER;
+  }
+
+  /**
+   * Subclass and override to return a SAX {@link ErrorHandler} to be used by {@link DocumentBuilder}.
+   * By default this returns {@code null} so no error handler is used.
+   * This method can be used to redirect XML parse errors/warnings to a custom logger.
+   */
+  protected ErrorHandler getErrorHandler() {
+    return null;
+  }
+
+  private Document parseXML(InputStream pXmlFile) throws ParserException {
+    final DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance();
+    dbf.setValidating(false);
     try {
-      db = dbf.newDocumentBuilder();
-    }
-    catch (Exception se) {
-      throw new ParserException("XML Parser configuration error", se);
+      dbf.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
+    } catch (ParserConfigurationException e) {
+      // ignore since all implementations are required to support the
+      // {@link javax.xml.XMLConstants#FEATURE_SECURE_PROCESSING} feature
     }
-    org.w3c.dom.Document doc = null;
+    final DocumentBuilder db;
     try {
-      doc = db.parse(pXmlFile);
+      db = dbf.newDocumentBuilder();
+    } catch (Exception se) {
+      throw new ParserException("XML Parser configuration error.", se);
     }
-    catch (Exception se) {
-      throw new ParserException("Error parsing XML stream:" + se, se);
+    try {
+      db.setEntityResolver(getEntityResolver());
+      db.setErrorHandler(getErrorHandler());
+      return db.parse(pXmlFile);
+    } catch (Exception se) {
+      throw new ParserException("Error parsing XML stream: " + se, se);
     }
-    return doc;
   }
 
   public Query getQuery(Element e) throws ParserException {
@@ -160,4 +194,11 @@ public Query getQuery(Element e) throws ParserException {
   public SpanQuery getSpanQuery(Element e) throws ParserException {
     return spanFactory.getSpanQuery(e);
   }
+
+  public static final EntityResolver DISALLOW_EXTERNAL_ENTITY_RESOLVER = (String publicId, String systemId) -> {
+    throw new SAXException(String.format(Locale.ENGLISH,
+        "External Entity resolving unsupported:  publicId=\"%s\" systemId=\"%s\"",
+        publicId, systemId));
+  };
+
 }

lucene/queryparser/src/test/org/apache/lucene/queryparser/xml/DOCTYPE_TermQuery.xml

@@ -0,0 +1,19 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!DOCTYPE TermQuery SYSTEM "foo://bar.xyz/mydtd">
+<!--
+ Licensed to the Apache Software Foundation (ASF) under one or more
+ contributor license agreements.  See the NOTICE file distributed with
+ this work for additional information regarding copyright ownership.
+ The ASF licenses this file to You under the Apache License, Version 2.0
+ (the "License"); you may not use this file except in compliance with
+ the License.  You may obtain a copy of the License at
+
+     http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+-->
+<TermQuery fieldName="contents">sumitomo</TermQuery>

lucene/queryparser/src/test/org/apache/lucene/queryparser/xml/ENTITY_TermQuery.xml

@@ -0,0 +1,23 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!DOCTYPE TermQuery [
+<!ENTITY internalTerm "sumitomo">
+<!ENTITY externalTerm SYSTEM "foo://bar.xyz/external">
+<!ENTITY % myParameterEntity "foo://bar.xyz/param">
+]>
+<!--
+ Licensed to the Apache Software Foundation (ASF) under one or more
+ contributor license agreements.  See the NOTICE file distributed with
+ this work for additional information regarding copyright ownership.
+ The ASF licenses this file to You under the Apache License, Version 2.0
+ (the "License"); you may not use this file except in compliance with
+ the License.  You may obtain a copy of the License at
+
+     http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+-->
+<TermQuery fieldName="contents">&internalTerm;&externalTerm;</TermQuery>

lucene/queryparser/src/test/org/apache/lucene/queryparser/xml/TestCoreParser.java

@@ -30,6 +30,7 @@
 import org.apache.lucene.search.spans.SpanQuery;
 import org.apache.lucene.util.LuceneTestCase;
 import org.junit.AfterClass;
+import org.xml.sax.SAXException;
 
 import java.io.IOException;
 import java.io.InputStream;
@@ -67,6 +68,18 @@ public void testTermQueryXML() throws ParserException, IOException {
     dumpResults("TermQuery", q, 5);
   }
 
+  public void test_DOCTYPE_TermQueryXML() throws ParserException, IOException {
+    SAXException saxe = LuceneTestCase.expectThrows(ParserException.class, SAXException.class,
+        () -> parse("DOCTYPE_TermQuery.xml"));
+    assertTrue(saxe.getMessage().startsWith("External Entity resolving unsupported:"));
+  }
+
+  public void test_ENTITY_TermQueryXML() throws ParserException, IOException {
+    SAXException saxe = LuceneTestCase.expectThrows(ParserException.class, SAXException.class,
+        () -> parse("ENTITY_TermQuery.xml"));
+    assertTrue(saxe.getMessage().startsWith("External Entity resolving unsupported:"));
+  }
+
   public void testTermQueryEmptyXML() throws ParserException, IOException {
     parseShouldFail("TermQueryEmpty.xml",
         "TermQuery has no text");

solr/CHANGES.txt

@@ -64,6 +64,9 @@ Upgrade Notes
   events caused by updates, commits, or optimize, write your own listener as native Java class
   as part of a Solr plugin.
 
+* SOLR-11477: in the XML query parser (defType=xmlparser or {!xmlparser ... })
+  the resolving of external entities is now disallowed by default.
+
 New Features
 ----------------------
 * SOLR-10339: New set-trigger and remove-trigger APIs for autoscaling. (shalin)
@@ -233,6 +236,9 @@ Bug Fixes
 
 * SOLR-11449: MoveReplicaCmd mistakenly called registerCollectionStateWatcher on failure. (ab)
 
+* SOLR-11477: Disallow resolving of external entities in the XML query parser (defType=xmlparser).
+  (Michael Stepankin, Olga Barinova, Uwe Schindler, Christine Poerschke)
+
 Optimizations
 ----------------------
 

solr/core/src/java/org/apache/solr/search/SolrCoreParser.java

@@ -25,11 +25,13 @@
 import org.apache.lucene.queryparser.xml.builders.SpanQueryBuilder;
 import org.apache.solr.common.SolrException;
 import org.apache.solr.common.util.NamedList;
+import org.apache.solr.common.util.XMLErrorLogger;
 import org.apache.solr.core.SolrResourceLoader;
 import org.apache.solr.request.SolrQueryRequest;
 import org.apache.solr.util.plugin.NamedListInitializedPlugin;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
+import org.xml.sax.ErrorHandler;
 
 /**
  * Assembles a QueryBuilder which uses Query objects from Solr's <code>search</code> module
@@ -38,6 +40,7 @@
 public class SolrCoreParser extends CoreParser implements NamedListInitializedPlugin {
 
   private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+  private static final XMLErrorLogger xmllog = new XMLErrorLogger(log);
 
   protected final SolrQueryRequest req;
 
@@ -97,4 +100,9 @@ public void init(NamedList initArgs) {
     }
   }
 
+  @Override
+  protected ErrorHandler getErrorHandler() {
+    return xmllog;
+  }
+
 }