Plugins: Enforce plugin zip does not contain zip entries outside of the unzip dir

When unzipping a plugin zip, the zip entries are resolved relative to
the directory being unzipped into. However, there are currently no
checks that the entry name was not absolute, or relatively points
outside of the plugin dir. This change adds a check for those two cases.
This commit is contained in:
Ryan Ernst 2016-03-11 14:53:14 -08:00
parent 5f3d0067f8
commit 8b26c260d1
2 changed files with 20 additions and 1 deletions

View File

@ -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?)

View File

@ -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...