SOLR-10031: Validation of filename params in ReplicationHandler

This commit is contained in:
Jan Høydahl 2017-01-29 19:42:41 +01:00
parent bb5bf3fbf7
commit 6f598d2469
3 changed files with 39 additions and 3 deletions

View File

@ -161,6 +161,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.

View File

@ -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;
@ -1418,9 +1420,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);
@ -1434,6 +1437,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);

View File

@ -1424,6 +1424,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;