[solr] branch branch_9x updated: SOLR-16075 ShowFile handler should validate that files param is strictly relative to instance dir in standalone mode (#725)

[ja...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

janhoy pushed a commit to branch branch_9x
in repository https://gitbox.apache.org/repos/asf/solr.git


The following commit(s) were added to refs/heads/branch_9x by this push:
     new a7ad826  SOLR-16075 ShowFile handler should validate that files param is strictly relative to instance dir in standalone mode (#725)
a7ad826 is described below

commit a7ad8265ea1a9971802e68b1014fc9fb960df3cf
Author: Jan Høydahl <ja...@users.noreply.github.com>
AuthorDate: Fri Mar 4 17:26:18 2022 +0100

    SOLR-16075 ShowFile handler should validate that files param is strictly relative to instance dir in standalone mode (#725)
    
    (cherry picked from commit 2479013589c6d056f5d0e83206d9880641117e00)
---
 solr/CHANGES.txt                                   |  2 ++
 .../solr/handler/admin/ShowFileRequestHandler.java |  6 ++++++
 .../handler/admin/ShowFileRequestHandlerTest.java  | 23 ++++++++++++++++++++--
 3 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index ee6390e..dfd062d 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -653,6 +653,8 @@ Bug Fixes
 
 * SOLR-15968: Hide annoying WARN log from bin/solr zk command (janhoy, Mike Drob)
 
+* SOLR-16075: ShowFileHandler path parameter is now validated to be relative to instance conf dir in standalone mode (janhoy)
+
 * SOLR-15558: Don't wait for zombie processes to exit when stopping. (Colvin Cowie)
 
 * SOLR-16019: UTF-8 parsing errors for parameters should cause a HTTP 400 status code, not 500 (janhoy, Matthias Pigulla)
diff --git a/solr/core/src/java/org/apache/solr/handler/admin/ShowFileRequestHandler.java b/solr/core/src/java/org/apache/solr/handler/admin/ShowFileRequestHandler.java
index 4747ff8..4e7e1b8 100644
--- a/solr/core/src/java/org/apache/solr/handler/admin/ShowFileRequestHandler.java
+++ b/solr/core/src/java/org/apache/solr/handler/admin/ShowFileRequestHandler.java
@@ -386,6 +386,12 @@ public class ShowFileRequestHandler extends RequestHandlerBase implements Permis
     // A leading slash is unnecessary but supported and interpreted as start of config dir
     Path filePath = configDir.resolve(fname.startsWith("/") ? fname.substring(1) : fname);
     req.getCoreContainer().assertPathAllowed(filePath);
+    if (!filePath.normalize().startsWith(configDir.normalize())) {
+      log.error("Path must be inside core config directory");
+      rsp.setException(
+          new SolrException(ErrorCode.BAD_REQUEST, "Path must be inside core config directory"));
+      return null;
+    }
     return filePath;
   }
 
diff --git a/solr/core/src/test/org/apache/solr/handler/admin/ShowFileRequestHandlerTest.java b/solr/core/src/test/org/apache/solr/handler/admin/ShowFileRequestHandlerTest.java
index 2a06b99..7a133eb 100644
--- a/solr/core/src/test/org/apache/solr/handler/admin/ShowFileRequestHandlerTest.java
+++ b/solr/core/src/test/org/apache/solr/handler/admin/ShowFileRequestHandlerTest.java
@@ -24,6 +24,7 @@ import org.apache.solr.SolrJettyTestBase;
 import org.apache.solr.client.solrj.ResponseParser;
 import org.apache.solr.client.solrj.SolrClient;
 import org.apache.solr.client.solrj.SolrServerException;
+import org.apache.solr.client.solrj.impl.BaseHttpSolrClient;
 import org.apache.solr.client.solrj.impl.NoOpResponseParser;
 import org.apache.solr.client.solrj.request.QueryRequest;
 import org.apache.solr.client.solrj.response.QueryResponse;
@@ -154,15 +155,33 @@ public class ShowFileRequestHandlerTest extends SolrJettyTestBase {
 
   public void testAbsoluteFilename() {
     SolrClient client = getSolrClient();
-    final QueryRequest request = new QueryRequest(params("file", "/etc/passwd"));
+    final QueryRequest request =
+        new QueryRequest(params("file", "/etc/passwd", "contentType", "text/plain; charset=utf-8"));
     request.setPath("/admin/file"); // absolute path not allowed
     request.setResponseParser(new NoOpResponseParser());
     expectThrows(SolrException.class, () -> client.request(request));
   }
 
+  public void testEscapeConfDir() {
+    SolrClient client = getSolrClient();
+    final QueryRequest request =
+        new QueryRequest(
+            params("file", "../../solr.xml", "contentType", "application/xml; charset=utf-8"));
+    request.setPath("/admin/file");
+    request.setResponseParser(new NoOpResponseParser());
+    var ex = expectThrows(SolrException.class, () -> client.request(request));
+    assertTrue(ex instanceof BaseHttpSolrClient.RemoteSolrException);
+  }
+
   public void testPathTraversalFilename() {
     SolrClient client = getSolrClient();
-    final QueryRequest request = new QueryRequest(params("file", "../../../../../../etc/passwd"));
+    final QueryRequest request =
+        new QueryRequest(
+            params(
+                "file",
+                "../../../../../../etc/passwd",
+                "contentType",
+                "text/plain; charset=utf-8"));
     request.setPath("/admin/file");
     request.setResponseParser(new NoOpResponseParser());
     expectThrows(SolrException.class, () -> client.request(request));