From a832987ada172d91baa03a779a21b7541ddd6857 Mon Sep 17 00:00:00 2001 From: Adrien Grand Date: Mon, 27 Jul 2015 16:03:11 +0200 Subject: [PATCH] Forbid Files.isHidden. As Robert pointed out on #12465, it has the undesirable property of relying on the operating system. So it would be better to use a simple rule such as checking whether the file name starts with a dot. --- .../metadata/MetaDataCreateIndexService.java | 3 +- .../common/io/FileSystemUtils.java | 16 +++++++++ .../org/elasticsearch/http/HttpServer.java | 5 +-- .../elasticsearch/plugins/PluginsService.java | 6 ++-- .../common/io/FileSystemUtilsTests.java | 36 +++++++++++++++++++ .../resources/forbidden/all-signatures.txt | 4 ++- 6 files changed, 62 insertions(+), 8 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexService.java b/core/src/main/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexService.java index 7f70886116d..ae6119ec46a 100644 --- a/core/src/main/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexService.java +++ b/core/src/main/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexService.java @@ -49,6 +49,7 @@ import org.elasticsearch.common.Strings; import org.elasticsearch.common.component.AbstractComponent; import org.elasticsearch.common.compress.CompressedXContent; import org.elasticsearch.common.inject.Inject; +import org.elasticsearch.common.io.FileSystemUtils; import org.elasticsearch.common.io.Streams; import org.elasticsearch.common.regex.Regex; import org.elasticsearch.common.settings.Settings; @@ -465,7 +466,7 @@ public class MetaDataCreateIndexService extends AbstractComponent { try (DirectoryStream stream = Files.newDirectoryStream(mappingsDir)) { for (Path mappingFile : stream) { final String fileName = mappingFile.getFileName().toString(); - if (Files.isHidden(mappingFile)) { + if (FileSystemUtils.isHidden(mappingFile)) { continue; } int lastDotIndex = fileName.lastIndexOf('.'); diff --git a/core/src/main/java/org/elasticsearch/common/io/FileSystemUtils.java b/core/src/main/java/org/elasticsearch/common/io/FileSystemUtils.java index 50f845f754a..bf7eb5ee28d 100644 --- a/core/src/main/java/org/elasticsearch/common/io/FileSystemUtils.java +++ b/core/src/main/java/org/elasticsearch/common/io/FileSystemUtils.java @@ -20,6 +20,7 @@ package org.elasticsearch.common.io; import com.google.common.collect.Iterators; + import org.apache.lucene.util.IOUtils; import org.elasticsearch.common.logging.ESLogger; @@ -84,6 +85,20 @@ public final class FileSystemUtils { return false; } + /** + * Check whether the file denoted by the given path is hidden. + * In practice, this will check if the file name starts with a dot. + * This should be preferred to {@link Files#isHidden(Path)} as this + * does not depend on the operating system. + */ + public static boolean isHidden(Path path) { + Path fileName = path.getFileName(); + if (fileName == null) { + return false; + } + return fileName.toString().startsWith("."); + } + /** * Appends the path to the given base and strips N elements off the path if strip is > 0. */ @@ -334,4 +349,5 @@ public final class FileSystemUtils { return Iterators.toArray(stream.iterator(), Path.class); } } + } diff --git a/core/src/main/java/org/elasticsearch/http/HttpServer.java b/core/src/main/java/org/elasticsearch/http/HttpServer.java index 5199ea5e422..40067e2bff3 100644 --- a/core/src/main/java/org/elasticsearch/http/HttpServer.java +++ b/core/src/main/java/org/elasticsearch/http/HttpServer.java @@ -23,6 +23,7 @@ import com.google.common.collect.ImmutableMap; import org.elasticsearch.common.component.AbstractLifecycleComponent; import org.elasticsearch.common.inject.Inject; +import org.elasticsearch.common.io.FileSystemUtils; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.env.Environment; import org.elasticsearch.node.service.NodeService; @@ -183,7 +184,7 @@ public class HttpServer extends AbstractLifecycleComponent { Path file = siteFile.resolve(sitePath); // return not found instead of forbidden to prevent malicious requests to find out if files exist or dont exist - if (!Files.exists(file) || Files.isHidden(file) || !file.toAbsolutePath().normalize().startsWith(siteFile.toAbsolutePath().normalize())) { + if (!Files.exists(file) || FileSystemUtils.isHidden(file) || !file.toAbsolutePath().normalize().startsWith(siteFile.toAbsolutePath().normalize())) { channel.sendResponse(new BytesRestResponse(NOT_FOUND)); return; } @@ -197,7 +198,7 @@ public class HttpServer extends AbstractLifecycleComponent { } // We don't serve dir but if index.html exists in dir we should serve it file = file.resolve("index.html"); - if (!Files.exists(file) || Files.isHidden(file) || !Files.isRegularFile(file)) { + if (!Files.exists(file) || FileSystemUtils.isHidden(file) || !Files.isRegularFile(file)) { channel.sendResponse(new BytesRestResponse(FORBIDDEN)); return; } diff --git a/core/src/main/java/org/elasticsearch/plugins/PluginsService.java b/core/src/main/java/org/elasticsearch/plugins/PluginsService.java index abb313c0a82..70494d71f82 100644 --- a/core/src/main/java/org/elasticsearch/plugins/PluginsService.java +++ b/core/src/main/java/org/elasticsearch/plugins/PluginsService.java @@ -23,7 +23,6 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import org.elasticsearch.ElasticsearchException; -import org.elasticsearch.Version; import org.elasticsearch.action.admin.cluster.node.info.PluginsInfo; import org.elasticsearch.bootstrap.Bootstrap; import org.elasticsearch.bootstrap.JarHell; @@ -33,6 +32,7 @@ import org.elasticsearch.common.collect.Tuple; import org.elasticsearch.common.component.AbstractComponent; import org.elasticsearch.common.component.LifecycleComponent; import org.elasticsearch.common.inject.Module; +import org.elasticsearch.common.io.FileSystemUtils; import org.elasticsearch.common.logging.ESLogger; import org.elasticsearch.common.logging.Loggers; import org.elasticsearch.common.settings.Settings; @@ -40,7 +40,6 @@ import org.elasticsearch.env.Environment; import java.io.Closeable; import java.io.IOException; -import java.io.InputStream; import java.lang.reflect.Method; import java.net.URL; import java.net.URLClassLoader; @@ -54,7 +53,6 @@ import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; -import java.util.Properties; import java.util.Set; import static org.elasticsearch.common.io.FileSystemUtils.isAccessibleDirectory; @@ -312,7 +310,7 @@ public class PluginsService extends AbstractComponent { try (DirectoryStream stream = Files.newDirectoryStream(pluginsDirectory)) { for (Path plugin : stream) { try { - if (Files.isHidden(plugin)) { + if (FileSystemUtils.isHidden(plugin)) { logger.trace("--- skip hidden plugin file[{}]", plugin.toAbsolutePath()); continue; } diff --git a/core/src/test/java/org/elasticsearch/common/io/FileSystemUtilsTests.java b/core/src/test/java/org/elasticsearch/common/io/FileSystemUtilsTests.java index aa8c56cc97a..53d7c82de30 100644 --- a/core/src/test/java/org/elasticsearch/common/io/FileSystemUtilsTests.java +++ b/core/src/test/java/org/elasticsearch/common/io/FileSystemUtilsTests.java @@ -32,6 +32,7 @@ import java.net.URISyntaxException; import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; +import java.util.Arrays; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertFileExists; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertFileNotExists; @@ -173,4 +174,39 @@ public class FileSystemUtilsTests extends ElasticsearchTestCase { assertEquals(FileSystemUtils.append(PathUtils.get("/foo/bar"), PathUtils.get("/hello/world/this_is/awesome"), 1), PathUtils.get("/foo/bar/world/this_is/awesome")); } + + public void testIsHidden() { + for (String p : Arrays.asList( + "/", + "foo", + "/foo", + "foo.bar", + "/foo.bar", + "foo/bar", + "foo/./bar", + "foo/../bar", + "/foo/./bar", + "/foo/../bar" + )) { + Path path = PathUtils.get(p); + assertFalse(FileSystemUtils.isHidden(path)); + } + for (String p : Arrays.asList( + ".hidden", + ".hidden.ext", + "/.hidden", + "/.hidden.ext", + "foo/.hidden", + "foo/.hidden.ext", + "/foo/.hidden", + "/foo/.hidden.ext", + ".", + "..", + "foo/.", + "foo/.." + )) { + Path path = PathUtils.get(p); + assertTrue(FileSystemUtils.isHidden(path)); + } + } } diff --git a/dev-tools/src/main/resources/forbidden/all-signatures.txt b/dev-tools/src/main/resources/forbidden/all-signatures.txt index cb4b2a43632..d33c4ee9b8b 100644 --- a/dev-tools/src/main/resources/forbidden/all-signatures.txt +++ b/dev-tools/src/main/resources/forbidden/all-signatures.txt @@ -60,4 +60,6 @@ com.google.common.collect.Iterators#emptyIterator() @ Use Collections.emptyItera java.io.ObjectOutputStream java.io.ObjectOutput java.io.ObjectInputStream -java.io.ObjectInput \ No newline at end of file +java.io.ObjectInput + +java.nio.file.Files#isHidden(java.nio.file.Path) @ Dependent on the operating system, use FileSystemUtils.isHidden instead