diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/submodule/SubmoduleAddTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/submodule/SubmoduleAddTest.java index 1a67e4197..0676eab2d 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/submodule/SubmoduleAddTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/submodule/SubmoduleAddTest.java @@ -136,7 +136,49 @@ public class SubmoduleAddTest extends RepositoryTestCase { } SubmoduleWalk generator = SubmoduleWalk.forIndex(db); + generator.loadModulesConfig(); assertTrue(generator.next()); + assertEquals(path, generator.getModuleName()); + assertEquals(path, generator.getPath()); + assertEquals(commit, generator.getObjectId()); + assertEquals(uri, generator.getModulesUrl()); + assertEquals(path, generator.getModulesPath()); + assertEquals(uri, generator.getConfigUrl()); + try (Repository subModRepo = generator.getRepository()) { + assertNotNull(subModRepo); + assertEquals(subCommit, commit); + } + + Status status = Git.wrap(db).status().call(); + assertTrue(status.getAdded().contains(Constants.DOT_GIT_MODULES)); + assertTrue(status.getAdded().contains(path)); + } + } + + @Test + public void addSubmoduleWithName() throws Exception { + try (Git git = new Git(db)) { + writeTrashFile("file.txt", "content"); + git.add().addFilepattern("file.txt").call(); + RevCommit commit = git.commit().setMessage("create file").call(); + + SubmoduleAddCommand command = new SubmoduleAddCommand(db); + String name = "testsub"; + command.setName(name); + String path = "sub"; + command.setPath(path); + String uri = db.getDirectory().toURI().toString(); + command.setURI(uri); + ObjectId subCommit; + try (Repository repo = command.call()) { + assertNotNull(repo); + subCommit = repo.resolve(Constants.HEAD); + } + + SubmoduleWalk generator = SubmoduleWalk.forIndex(db); + generator.loadModulesConfig(); + assertTrue(generator.next()); + assertEquals(name, generator.getModuleName()); assertEquals(path, generator.getPath()); assertEquals(commit, generator.getObjectId()); assertEquals(uri, generator.getModulesUrl()); @@ -268,4 +310,18 @@ public class SubmoduleAddTest extends RepositoryTestCase { ConfigConstants.CONFIG_KEY_URL)); } } + + @Test + public void denySubmoduleWithDotDot() throws Exception { + SubmoduleAddCommand command = new SubmoduleAddCommand(db); + command.setName("dir/../"); + command.setPath("sub"); + command.setURI(db.getDirectory().toURI().toString()); + try { + command.call(); + fail(); + } catch (IllegalArgumentException e) { + // Expected + } + } } diff --git a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties index ee5ba20b2..34c90f977 100644 --- a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties +++ b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties @@ -376,6 +376,7 @@ invalidLineInConfigFile=Invalid line in config file invalidLineInConfigFileWithParam=Invalid line in config file: {0} invalidModeFor=Invalid mode {0} for {1} {2} in {3}. invalidModeForPath=Invalid mode {0} for path {1} +invalidNameContainsDotDot=Invalid name (contains ".."): {1} invalidObject=Invalid {0} {1}: {2} invalidOldIdSent=invalid old id sent invalidPacketLineHeader=Invalid packet line header: {0} diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/api/SubmoduleAddCommand.java b/org.eclipse.jgit/src/org/eclipse/jgit/api/SubmoduleAddCommand.java index 9538d2be6..b8d4468a2 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/api/SubmoduleAddCommand.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/api/SubmoduleAddCommand.java @@ -76,6 +76,8 @@ import org.eclipse.jgit.treewalk.filter.TreeFilter; public class SubmoduleAddCommand extends TransportCommand { + private String name; + private String path; private String uri; @@ -92,6 +94,18 @@ public class SubmoduleAddCommand extends super(repo); } + /** + * Set the submodule name + * + * @param name + * @return this command + * @since 5.1 + */ + public SubmoduleAddCommand setName(String name) { + this.name = name; + return this; + } + /** * Set repository-relative path of submodule * @@ -160,6 +174,25 @@ public class SubmoduleAddCommand extends throw new IllegalArgumentException(JGitText.get().pathNotConfigured); if (uri == null || uri.length() == 0) throw new IllegalArgumentException(JGitText.get().uriNotConfigured); + if (name == null || name.length() == 0) { + // Use the path as the default. + name = path; + } + if (name.contains("/../") || name.contains("\\..\\") //$NON-NLS-1$ //$NON-NLS-2$ + || name.startsWith("../") || name.startsWith("..\\") //$NON-NLS-1$ //$NON-NLS-2$ + || name.endsWith("/..") || name.endsWith("\\..")) { //$NON-NLS-1$ //$NON-NLS-2$ + // Submodule names are used to store the submodule repositories + // under $GIT_DIR/modules. Having ".." in submodule names makes a + // vulnerability (CVE-2018-11235 + // https://bugs.eclipse.org/bugs/show_bug.cgi?id=535027#c0) + // Reject the names with them. The callers need to make sure the + // names free from these. We don't automatically replace these + // characters or canonicalize by regarding the name as a file path. + // Since Path class is platform dependent, we manually check '/' and + // '\\' patterns here. + throw new IllegalArgumentException(MessageFormat + .format(JGitText.get().invalidNameContainsDotDot, name)); + } try { if (submoduleExists()) @@ -193,7 +226,7 @@ public class SubmoduleAddCommand extends // Save submodule URL to parent repository's config StoredConfig config = repo.getConfig(); - config.setString(ConfigConstants.CONFIG_SUBMODULE_SECTION, path, + config.setString(ConfigConstants.CONFIG_SUBMODULE_SECTION, name, ConfigConstants.CONFIG_KEY_URL, resolvedUri); try { config.save(); @@ -207,9 +240,9 @@ public class SubmoduleAddCommand extends try { modulesConfig.load(); modulesConfig.setString(ConfigConstants.CONFIG_SUBMODULE_SECTION, - path, ConfigConstants.CONFIG_KEY_PATH, path); + name, ConfigConstants.CONFIG_KEY_PATH, path); modulesConfig.setString(ConfigConstants.CONFIG_SUBMODULE_SECTION, - path, ConfigConstants.CONFIG_KEY_URL, uri); + name, ConfigConstants.CONFIG_KEY_URL, uri); modulesConfig.save(); } catch (IOException e) { throw new JGitInternalException(e.getMessage(), e); diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java index 7c29d4d13..69181881e 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java @@ -437,6 +437,7 @@ public class JGitText extends TranslationBundle { /***/ public String invalidLineInConfigFileWithParam; /***/ public String invalidModeFor; /***/ public String invalidModeForPath; + /***/ public String invalidNameContainsDotDot; /***/ public String invalidObject; /***/ public String invalidOldIdSent; /***/ public String invalidPacketLineHeader;