Useful for validating user-provided relative paths, which generally aren't expected to + * "escape" a given parent/root directory. Parent and child paths are "normalized" by {@link + * Path#normalize()}. This removes explicit backtracking (e.g. "../") though it will not resolve + * symlinks if any are present in the provided Paths, so some forms of parent "escape" remain + * undetected. Paths needn't exist as a file or directory for comparison purposes. + * + *
Note, this method does not consult the file system + * + * @param parent the path of a 'parent' node. Path must be syntactically valid but needn't exist. + * @param potentialChild the path of a potential child. Typically obtained via: + * parent.resolve(relativeChildPath). Path must be syntactically valid but needn't exist. + * @return true if 'potentialChild' nests under the provided 'parent', false otherwise. + */ + public static boolean isPathAChildOfParent(Path parent, Path potentialChild) { + final var normalizedParent = parent.toAbsolutePath().normalize(); + final var normalizedChild = potentialChild.toAbsolutePath().normalize(); + + return normalizedChild.startsWith(normalizedParent) + && !normalizedChild.equals(normalizedParent); + } } diff --git a/solr/core/src/test/org/apache/solr/core/TestFileSystemConfigSetService.java b/solr/core/src/test/org/apache/solr/core/TestFileSystemConfigSetService.java index f9695a990e5..e4ceb1b831c 100644 --- a/solr/core/src/test/org/apache/solr/core/TestFileSystemConfigSetService.java +++ b/solr/core/src/test/org/apache/solr/core/TestFileSystemConfigSetService.java @@ -17,7 +17,9 @@ package org.apache.solr.core; import static org.apache.solr.core.FileSystemConfigSetService.METADATA_FILE; +import static org.hamcrest.Matchers.hasItem; +import java.io.File; import java.io.IOException; import java.nio.charset.StandardCharsets; import java.nio.file.Files; @@ -49,13 +51,43 @@ public static void afterClass() throws Exception { fileSystemConfigSetService = null; } + @Test + public void testIgnoresFileUploadsOutsideOfConfigSetDirectory() throws IOException { + final var initialNumConfigs = fileSystemConfigSetService.listConfigs().size(); + final String configName = "fileEscapeTestConfig"; + final var specificConfigSetBase = configSetBase.resolve(configName); + + fileSystemConfigSetService.uploadConfig(configName, configset("cloud-minimal")); + assertEquals(fileSystemConfigSetService.listConfigs().size(), initialNumConfigs + 1); + assertTrue(fileSystemConfigSetService.checkConfigExists(configName)); + + // This succeeds, as the file is an allowed type and the path doesn't attempt to escape the + // configset's root + byte[] testdata = "test data".getBytes(StandardCharsets.UTF_8); + fileSystemConfigSetService.uploadFileToConfig(configName, "validPath", testdata, true); + final var knownFiles = fileSystemConfigSetService.getAllConfigFiles(configName); + assertThat(knownFiles, hasItem("validPath")); + assertTrue(Files.exists(specificConfigSetBase.resolve("validPath"))); + + // Each of these will fail "quietly" as ConfigSetService opts to log warnings but otherwise not + // surface validation errors to enable bulk uploading + final var invalidFilePaths = + List.of( + ".." + File.separator + "escapePath", + "foo" + File.separator + ".." + File.separator + ".." + File.separator + "bar"); + for (String invalidFilePath : invalidFilePaths) { + fileSystemConfigSetService.uploadFileToConfig(configName, invalidFilePath, testdata, true); + assertFalse(Files.exists(specificConfigSetBase.resolve(invalidFilePath))); + } + } + @Test public void testUploadAndDeleteConfig() throws IOException { + final var initialNumConfigs = fileSystemConfigSetService.listConfigs().size(); String configName = "testconfig"; fileSystemConfigSetService.uploadConfig(configName, configset("cloud-minimal")); - - assertEquals(fileSystemConfigSetService.listConfigs().size(), 1); + assertEquals(fileSystemConfigSetService.listConfigs().size(), initialNumConfigs + 1); assertTrue(fileSystemConfigSetService.checkConfigExists(configName)); byte[] testdata = "test data".getBytes(StandardCharsets.UTF_8); @@ -79,7 +111,7 @@ public void testUploadAndDeleteConfig() throws IOException { assertEquals("[schema.xml, solrconfig.xml]", allConfigFiles.toString()); fileSystemConfigSetService.copyConfig(configName, "copytestconfig"); - assertEquals(fileSystemConfigSetService.listConfigs().size(), 2); + assertEquals(fileSystemConfigSetService.listConfigs().size(), initialNumConfigs + 2); allConfigFiles = fileSystemConfigSetService.getAllConfigFiles("copytestconfig"); assertEquals("[schema.xml, solrconfig.xml]", allConfigFiles.toString()); diff --git a/solr/core/src/test/org/apache/solr/util/FileUtilsTest.java b/solr/core/src/test/org/apache/solr/util/FileUtilsTest.java index 671157c9e72..0dfeff109a3 100644 --- a/solr/core/src/test/org/apache/solr/util/FileUtilsTest.java +++ b/solr/core/src/test/org/apache/solr/util/FileUtilsTest.java @@ -17,10 +17,13 @@ package org.apache.solr.util; import java.io.File; +import java.nio.file.Path; import org.apache.solr.SolrTestCase; +import org.junit.Test; public class FileUtilsTest extends SolrTestCase { + @Test public void testResolve() { String cwd = new File(".").getAbsolutePath(); assertEquals(new File("conf/data"), FileUtils.resolvePath(new File("conf"), "data")); @@ -28,4 +31,51 @@ public void testResolve() { new File(cwd + "/conf/data"), FileUtils.resolvePath(new File(cwd + "/conf"), "data")); assertEquals(new File(cwd + "/data"), FileUtils.resolvePath(new File("conf"), cwd + "/data")); } + + @Test + public void testDetectsPathEscape() { + final var parent = Path.of("."); + + // Allows simple child + assertTrue(FileUtils.isPathAChildOfParent(parent, parent.resolve("child"))); + + // Allows "./" prefixed child + assertTrue(FileUtils.isPathAChildOfParent(parent, parent.resolve(buildPath(".", "child")))); + + // Allows nested child + assertTrue( + FileUtils.isPathAChildOfParent(parent, parent.resolve(buildPath("nested", "child")))); + + // Allows backtracking, provided it stays "under" parent + assertTrue( + FileUtils.isPathAChildOfParent( + parent, parent.resolve(buildPath("child1", "..", "child2")))); + assertTrue( + FileUtils.isPathAChildOfParent( + parent, parent.resolve(buildPath("child", "grandchild1", "..", "grandchild2")))); + + // Prevents identical path + assertFalse(FileUtils.isPathAChildOfParent(parent, parent)); + + // Detects sibling of parent + assertFalse(FileUtils.isPathAChildOfParent(parent, parent.resolve(buildPath("..", "sibling")))); + + // Detects "grandparent" of parent + assertFalse(FileUtils.isPathAChildOfParent(parent, parent.resolve(".."))); + + // Detects many-layered backtracking + assertFalse( + FileUtils.isPathAChildOfParent(parent, parent.resolve(buildPath("..", "..", "..", "..")))); + } + + private static String buildPath(String... pathSegments) { + final var sb = new StringBuilder(); + for (int i = 0; i < pathSegments.length; i++) { + sb.append(pathSegments[i]); + if (i < pathSegments.length - 1) { + sb.append(File.separator); + } + } + return sb.toString(); + } }