diff --git a/core/src/main/java/org/elasticsearch/plugins/InstallPluginCommand.java b/core/src/main/java/org/elasticsearch/plugins/InstallPluginCommand.java index 32c4bf18507..e72eb2100f6 100644 --- a/core/src/main/java/org/elasticsearch/plugins/InstallPluginCommand.java +++ b/core/src/main/java/org/elasticsearch/plugins/InstallPluginCommand.java @@ -250,7 +250,14 @@ class InstallPluginCommand extends Command { } hasEsDir = true; Path targetFile = target.resolve(entry.getName().substring("elasticsearch/".length())); - // TODO: handle name being an absolute path + + // Using the entry name as a path can result in an entry outside of the plugin dir, either if the + // name starts with the root of the filesystem, or it is a relative entry like ../whatever. + // This check attempts to identify both cases by first normalizing the path (which removes foo/..) + // and ensuring the normalized entry is still rooted with the target plugin directory. + if (targetFile.normalize().startsWith(target) == false) { + throw new IOException("Zip contains entry name '" + entry.getName() + "' resolving outside of plugin directory"); + } // be on the safe side: do not rely on that directories are always extracted // before their children (although this makes sense, but is it guaranteed?) diff --git a/qa/evil-tests/src/test/java/org/elasticsearch/plugins/InstallPluginCommandTests.java b/qa/evil-tests/src/test/java/org/elasticsearch/plugins/InstallPluginCommandTests.java index 514090d9869..fb69c817f3a 100644 --- a/qa/evil-tests/src/test/java/org/elasticsearch/plugins/InstallPluginCommandTests.java +++ b/qa/evil-tests/src/test/java/org/elasticsearch/plugins/InstallPluginCommandTests.java @@ -466,6 +466,18 @@ public class InstallPluginCommandTests extends ESTestCase { assertInstallCleaned(env); } + public void testZipRelativeOutsideEntryName() throws Exception { + Path zip = createTempDir().resolve("broken.zip"); + try (ZipOutputStream stream = new ZipOutputStream(Files.newOutputStream(zip))) { + stream.putNextEntry(new ZipEntry("elasticsearch/../blah")); + } + String pluginZip = zip.toUri().toURL().toString(); + IOException e = expectThrows(IOException.class, () -> { + installPlugin(pluginZip, createEnv()); + }); + assertTrue(e.getMessage(), e.getMessage().contains("resolving outside of plugin directory")); + } + // TODO: test batch flag? // TODO: test checksum (need maven/official below) // TODO: test maven, official, and staging install...need tests with fixtures...