You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by ja...@apache.org on 2017/01/29 21:21:21 UTC
lucene-solr:branch_6x: SOLR-10031: Validation of filename params in
ReplicationHandler
Repository: lucene-solr
Updated Branches:
refs/heads/branch_6x 71a198ce3 -> 7088137d5
SOLR-10031: Validation of filename params in ReplicationHandler
(cherry picked from commit 6f598d2)
Project: http://git-wip-us.apache.org/repos/asf/lucene-solr/repo
Commit: http://git-wip-us.apache.org/repos/asf/lucene-solr/commit/7088137d
Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/7088137d
Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/7088137d
Branch: refs/heads/branch_6x
Commit: 7088137d52256354a52ed86547b9faa0e7042934
Parents: 71a198c
Author: Jan H�ydahl <ja...@apache.org>
Authored: Sun Jan 29 19:42:41 2017 +0100
Committer: Jan H�ydahl <ja...@apache.org>
Committed: Sun Jan 29 20:00:56 2017 +0100
----------------------------------------------------------------------
solr/CHANGES.txt | 2 ++
.../apache/solr/handler/ReplicationHandler.java | 25 +++++++++++++++++---
.../solr/handler/TestReplicationHandler.java | 15 ++++++++++++
3 files changed, 39 insertions(+), 3 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/7088137d/solr/CHANGES.txt
----------------------------------------------------------------------
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 7d487fa..dcb55ec 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -93,6 +93,8 @@ Bug Fixes
* SOLR-9969: "Plugin/Stats" section of the UI doesn't display empty metric types (Tom�s Fern�ndez L�bbe)
* SOLR-8491: solr.cmd SOLR_SSL_OPTS is overwritten (Sam Yi, Andy Hind, Marcel Berteler, Kevin Risden)
+* SOLR-10031: Validation of filename params in ReplicationHandler (Hrishikesh Gadre, janhoy)
+
================== 6.4.0 ==================
Consult the LUCENE_CHANGES.txt file for additional, low level, changes in this release.
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/7088137d/solr/core/src/java/org/apache/solr/handler/ReplicationHandler.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/handler/ReplicationHandler.java b/solr/core/src/java/org/apache/solr/handler/ReplicationHandler.java
index e6bea65..76bcae9 100644
--- a/solr/core/src/java/org/apache/solr/handler/ReplicationHandler.java
+++ b/solr/core/src/java/org/apache/solr/handler/ReplicationHandler.java
@@ -29,6 +29,8 @@ import java.nio.ByteBuffer;
import java.nio.channels.FileChannel;
import java.nio.charset.StandardCharsets;
import java.nio.file.NoSuchFileException;
+import java.nio.file.Path;
+import java.nio.file.Paths;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
@@ -1413,9 +1415,10 @@ public class ReplicationHandler extends RequestHandlerBase implements SolrCoreAw
params = solrParams;
delPolicy = core.getDeletionPolicy();
- fileName = params.get(FILE);
- cfileName = params.get(CONF_FILE_SHORT);
- tlogFileName = params.get(TLOG_FILE);
+ fileName = validateFilenameOrError(params.get(FILE));
+ cfileName = validateFilenameOrError(params.get(CONF_FILE_SHORT));
+ tlogFileName = validateFilenameOrError(params.get(TLOG_FILE));
+
sOffset = params.get(OFFSET);
sLen = params.get(LEN);
compress = params.get(COMPRESSION);
@@ -1429,6 +1432,22 @@ public class ReplicationHandler extends RequestHandlerBase implements SolrCoreAw
rateLimiter = new RateLimiter.SimpleRateLimiter(maxWriteMBPerSec);
}
+ // Throw exception on directory traversal attempts
+ protected String validateFilenameOrError(String filename) {
+ if (filename != null) {
+ Path filePath = Paths.get(filename);
+ filePath.forEach(subpath -> {
+ if ("..".equals(subpath.toString())) {
+ throw new SolrException(ErrorCode.FORBIDDEN, "File name cannot contain ..");
+ }
+ });
+ if (filePath.isAbsolute()) {
+ throw new SolrException(ErrorCode.FORBIDDEN, "File name must be relative");
+ }
+ return filename;
+ } else return null;
+ }
+
protected void initWrite() throws IOException {
if (sOffset != null) offset = Long.parseLong(sOffset);
if (sLen != null) len = Integer.parseInt(sLen);
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/7088137d/solr/core/src/test/org/apache/solr/handler/TestReplicationHandler.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/handler/TestReplicationHandler.java b/solr/core/src/test/org/apache/solr/handler/TestReplicationHandler.java
index 0c95baf..e6501ef 100644
--- a/solr/core/src/test/org/apache/solr/handler/TestReplicationHandler.java
+++ b/solr/core/src/test/org/apache/solr/handler/TestReplicationHandler.java
@@ -1426,6 +1426,21 @@ public class TestReplicationHandler extends SolrTestCaseJ4 {
assertTrue(timeTakenInSeconds - approximateTimeInSeconds > 0);
}
+ @Test
+ public void doTestIllegalFilePaths() throws Exception {
+ // Loop through the file=, cf=, tlogFile= params and prove that it throws exception for path traversal attempts
+ List<String> illegalFilenames = Arrays.asList("/foo/bar", "../dir/traversal", "illegal\rfile\nname\t");
+ List<String> params = Arrays.asList(ReplicationHandler.FILE, ReplicationHandler.CONF_FILE_SHORT, ReplicationHandler.TLOG_FILE);
+ for (String param : params) {
+ for (String filename : illegalFilenames) {
+ try {
+ invokeReplicationCommand(masterJetty.getLocalPort(), "filecontent&" + param + "=" + filename);
+ fail("Should have thrown exception on illegal path for param " + param + " and file name " + filename);
+ } catch (Exception e) {}
+ }
+ }
+ }
+
private class AddExtraDocs implements Runnable {
SolrClient masterClient;