diff --git a/src/main/java/org/elasticsearch/http/HttpServer.java b/src/main/java/org/elasticsearch/http/HttpServer.java index 6d43053e408..6a84b0c04dc 100644 --- a/src/main/java/org/elasticsearch/http/HttpServer.java +++ b/src/main/java/org/elasticsearch/http/HttpServer.java @@ -177,11 +177,13 @@ public class HttpServer extends AbstractLifecycleComponent { sitePath = sitePath.replace("/", separator); // this is a plugin provided site, serve it as static files from the plugin location Path file = FileSystemUtils.append(siteFile, PathUtils.get(sitePath), 0); - if (!Files.exists(file) || Files.isHidden(file)) { + + // 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())) { channel.sendResponse(new BytesRestResponse(NOT_FOUND)); return; } - + BasicFileAttributes attributes = Files.readAttributes(file, BasicFileAttributes.class); if (!attributes.isRegularFile()) { // If it's not a dir, we send a 403 @@ -196,10 +198,7 @@ public class HttpServer extends AbstractLifecycleComponent { return; } } - if (!file.toAbsolutePath().startsWith(siteFile.toAbsolutePath())) { - channel.sendResponse(new BytesRestResponse(FORBIDDEN)); - return; - } + try { byte[] data = Files.readAllBytes(file); channel.sendResponse(new BytesRestResponse(OK, guessMimeType(sitePath), data)); diff --git a/src/test/java/org/elasticsearch/plugins/SitePluginTests.java b/src/test/java/org/elasticsearch/plugins/SitePluginTests.java index 7889ec9ead9..8106c6f60dd 100644 --- a/src/test/java/org/elasticsearch/plugins/SitePluginTests.java +++ b/src/test/java/org/elasticsearch/plugins/SitePluginTests.java @@ -34,6 +34,9 @@ import org.junit.Test; import java.net.URISyntaxException; import java.nio.file.Path; import java.nio.file.Paths; +import java.util.ArrayList; +import java.util.List; +import java.util.Locale; import static org.elasticsearch.common.settings.ImmutableSettings.settingsBuilder; import static org.elasticsearch.test.ElasticsearchIntegrationTest.Scope; @@ -86,6 +89,34 @@ public class SitePluginTests extends ElasticsearchIntegrationTest { assertThat(response.getBody(), containsString("Dummy Site Plugin")); } + /** + * Test normalizing of path + */ + @Test + public void testThatPathsAreNormalized() throws Exception { + // more info: https://www.owasp.org/index.php/Path_Traversal + List notFoundUris = new ArrayList<>(); + notFoundUris.add("/_plugin/dummy/../../../../../log4j.properties"); + notFoundUris.add("/_plugin/dummy/../../../../../%00log4j.properties"); + notFoundUris.add("/_plugin/dummy/..%c0%af..%c0%af..%c0%af..%c0%af..%c0%aflog4j.properties"); + notFoundUris.add("/_plugin/dummy/%2E%2E/%2E%2E/%2E%2E/%2E%2E/index.html"); + notFoundUris.add("/_plugin/dummy/%2e%2e/%2e%2e/%2e%2e/%2e%2e/index.html"); + notFoundUris.add("/_plugin/dummy/%2e%2e%2f%2e%2e%2f%2e%2e%2f%2e%2e%2findex.html"); + notFoundUris.add("/_plugin/dummy/%2E%2E/%2E%2E/%2E%2E/%2E%2E/index.html"); + notFoundUris.add("/_plugin/dummy/..\\..\\..\\..\\..\\log4j.properties"); + + for (String uri : notFoundUris) { + HttpResponse response = httpClient().path(uri).execute(); + String message = String.format(Locale.ROOT, "URI [%s] expected to be not found", uri); + assertThat(message, response.getStatusCode(), equalTo(RestStatus.NOT_FOUND.getStatus())); + } + + // using relative path inside of the plugin should work + HttpResponse response = httpClient().path("/_plugin/dummy/dir1/../dir1/../index.html").execute(); + assertThat(response.getStatusCode(), equalTo(RestStatus.OK.getStatus())); + assertThat(response.getBody(), containsString("Dummy Site Plugin")); + } + /** * Test case for #4845: https://github.com/elasticsearch/elasticsearch/issues/4845 * Serving _site plugins do not pick up on index.html for sub directories diff --git a/src/test/resources/org/elasticsearch/plugins/dummy/_site/dir1/.empty b/src/test/resources/org/elasticsearch/plugins/dummy/_site/dir1/.empty new file mode 100644 index 00000000000..e69de29bb2d