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.
This commit is contained in:
Adrien Grand 2015-07-27 16:03:11 +02:00
parent a14913f7b6
commit a832987ada
6 changed files with 62 additions and 8 deletions

View File

@ -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<Path> 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('.');

View File

@ -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);
}
}
}

View File

@ -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<HttpServer> {
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<HttpServer> {
}
// 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;
}

View File

@ -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<Path> 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;
}

View File

@ -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));
}
}
}

View File

@ -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
java.io.ObjectInput
java.nio.file.Files#isHidden(java.nio.file.Path) @ Dependent on the operating system, use FileSystemUtils.isHidden instead