This is an automated email from the ASF dual-hosted git repository. tomaswolf pushed a commit to branch rc-2.18.0 in repository https://gitbox.apache.org/repos/asf/mina-sshd.git
commit db0567b33ce0d76750e7b7b7818794e78a3b17e3 Author: Thomas Wolf <[email protected]> AuthorDate: Sat May 9 12:22:36 2026 +0200 Improve git access --- .../org/apache/sshd/git/AbstractGitCommand.java | 39 ++++ .../org/apache/sshd/git/pack/GitPackCommand.java | 8 +- .../apache/sshd/git/pgm/EmbeddedCommandRunner.java | 3 +- .../apache/sshd/git/pack/GitPackCommandTest.java | 246 +++++++++++++++++++++ 4 files changed, 288 insertions(+), 8 deletions(-) diff --git a/sshd-git/src/main/java/org/apache/sshd/git/AbstractGitCommand.java b/sshd-git/src/main/java/org/apache/sshd/git/AbstractGitCommand.java index 7aa60b43f..2127e1c01 100644 --- a/sshd-git/src/main/java/org/apache/sshd/git/AbstractGitCommand.java +++ b/sshd-git/src/main/java/org/apache/sshd/git/AbstractGitCommand.java @@ -19,12 +19,17 @@ package org.apache.sshd.git; +import java.io.IOException; import java.io.OutputStream; +import java.nio.file.InvalidPathException; +import java.nio.file.Path; +import java.nio.file.Paths; import java.util.ArrayList; import java.util.List; import java.util.Objects; import org.apache.sshd.common.channel.ChannelOutputStream; +import org.apache.sshd.common.util.GenericUtils; import org.apache.sshd.common.util.threads.CloseableExecutorService; import org.apache.sshd.server.command.AbstractFileSystemCommand; @@ -139,4 +144,38 @@ public abstract class AbstractGitCommand return list; } + + public static Path resolveGitRepo(Path serverRoot, String pathArg) throws IOException { + String originalPath = pathArg; + int len = GenericUtils.length(pathArg); + // Strip any leading path separator since we use relative to root + if ((len > 0) && (pathArg.charAt(0) == '/')) { + pathArg = pathArg.substring(1); + } + Path gitRepoPath; + try { + gitRepoPath = Paths.get(pathArg); + } catch (InvalidPathException e) { + throw new IOException("Invalid git repository path " + originalPath, e); + } + if (gitRepoPath.isAbsolute()) { + throw new IOException("Absolute git repository path not allowed: " + originalPath); + } + // Remove "." and ".." components + gitRepoPath = gitRepoPath.normalize(); + int componentCount = gitRepoPath.getNameCount(); + // Deny access to the server root directory itself. + if (componentCount == 0 || gitRepoPath.toString().isEmpty()) { + throw new IOException("Invalid git repository path " + originalPath); + } + // The first component might still be "." or "..". Double check all components. + for (int i = 0; i < componentCount; i++) { + String component = gitRepoPath.getName(i).toString(); + if (".".equals(component) || "..".equals(component)) { + throw new IOException("Invalid git repository path " + originalPath); + } + } + return serverRoot.resolve(gitRepoPath); + + } } diff --git a/sshd-git/src/main/java/org/apache/sshd/git/pack/GitPackCommand.java b/sshd-git/src/main/java/org/apache/sshd/git/pack/GitPackCommand.java index 4d5e04584..bd2068410 100644 --- a/sshd-git/src/main/java/org/apache/sshd/git/pack/GitPackCommand.java +++ b/sshd-git/src/main/java/org/apache/sshd/git/pack/GitPackCommand.java @@ -115,14 +115,8 @@ public class GitPackCommand extends AbstractGitCommand { ValidateUtils.checkState(rootDir != null, "No root directory provided for %s command", command); String pathArg = args[1]; - int len = GenericUtils.length(pathArg); - // Strip any leading path separator since we use relative to root - if ((len > 0) && (pathArg.charAt(0) == '/')) { - pathArg = pathArg.substring(1); - } - ValidateUtils.hasContent(pathArg, "No %s command sub-path specified", args[0]); - return rootDir.resolve(pathArg); + return resolveGitRepo(rootDir, pathArg); } public void setPackConfiguration(GitPackConfiguration packConfiguration) { diff --git a/sshd-git/src/main/java/org/apache/sshd/git/pgm/EmbeddedCommandRunner.java b/sshd-git/src/main/java/org/apache/sshd/git/pgm/EmbeddedCommandRunner.java index b1ab09386..a6296ae90 100644 --- a/sshd-git/src/main/java/org/apache/sshd/git/pgm/EmbeddedCommandRunner.java +++ b/sshd-git/src/main/java/org/apache/sshd/git/pgm/EmbeddedCommandRunner.java @@ -32,6 +32,7 @@ import java.util.ArrayList; import java.util.List; import java.util.Objects; +import org.apache.sshd.git.AbstractGitCommand; import org.eclipse.jgit.api.Git; import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.pgm.CommandCatalog; @@ -128,7 +129,7 @@ public class EmbeddedCommandRunner { throw new Die(true); } - gitdir = Objects.toString(rootDir.resolve(gitdir)); + gitdir = Objects.toString(AbstractGitCommand.resolveGitRepo(rootDir, gitdir)); TextBuiltin cmd = subcommand; set(cmd, "ins", in); diff --git a/sshd-git/src/test/java/org/apache/sshd/git/pack/GitPackCommandTest.java b/sshd-git/src/test/java/org/apache/sshd/git/pack/GitPackCommandTest.java index 3e6dbe63d..b7686375a 100644 --- a/sshd-git/src/test/java/org/apache/sshd/git/pack/GitPackCommandTest.java +++ b/sshd-git/src/test/java/org/apache/sshd/git/pack/GitPackCommandTest.java @@ -144,6 +144,252 @@ public class GitPackCommandTest extends GitTestSupport { } } + @Test + void parentCheck() throws Exception { + Assumptions.assumeFalse(OsUtils.isWin32(), "On windows this activates TortoisePlink"); + + Path gitRootDir = getTempTargetRelativeFile(getClass().getSimpleName()); + SystemReader defaultSystemReader = mockGitConfig(gitRootDir); + try (SshServer sshd = setupTestServer()) { + GitPackTestConfig packConfig = new GitPackTestConfig(); + Path serverRootDir = gitRootDir.resolve("server"); + Path otherRootDir = gitRootDir.resolve("other"); + sshd.setSubsystemFactories(Collections.singletonList(new SftpSubsystemFactory())); + sshd.setCommandFactory(new GitPackCommandFactory(GitLocationResolver.constantPath(serverRootDir)) + .withGitPackConfiguration(packConfig)); + sshd.start(); + + int port = sshd.getPort(); + try { + CommonTestSupportUtils.deleteRecursive(serverRootDir); + Files.createDirectory(serverRootDir); + String repoName = getCurrentTestName() + Constants.DOT_GIT_EXT; + Path otherDir = otherRootDir.resolve(repoName); + CommonTestSupportUtils.deleteRecursive(otherDir); + Git.init().setBare(true).setDirectory(otherDir.toFile()).call().close(); + + JSch.setConfig("StrictHostKeyChecking", "no"); + CredentialsProvider + .setDefault(new UsernamePasswordCredentialsProvider(getCurrentTestName(), getCurrentTestName())); + Path localRootDir = gitRootDir.resolve("local"); + Path localDir = localRootDir.resolve(repoName); + CommonTestSupportUtils.deleteRecursive(localDir); + + SshClient client = SshClient.setUpDefaultClient(); + SshSessionFactory.setInstance(new GitSshdSessionFactory(client)); + try { + assertThrows(Exception.class, () -> { + Git.cloneRepository().setURI( + "ssh://" + getCurrentTestName() + "@" + TEST_LOCALHOST + ":" + port + "/../other/" + repoName) + .setDirectory(localDir.toFile()).call().close(); + }); + } finally { + client.stop(); + } + } finally { + sshd.stop(); + } + } finally { + SystemReader.setInstance(defaultSystemReader); + } + } + + @Test + void parentCheck2() throws Exception { + Assumptions.assumeFalse(OsUtils.isWin32(), "On windows this activates TortoisePlink"); + + Path gitRootDir = getTempTargetRelativeFile(getClass().getSimpleName()); + SystemReader defaultSystemReader = mockGitConfig(gitRootDir); + try (SshServer sshd = setupTestServer()) { + GitPackTestConfig packConfig = new GitPackTestConfig(); + Path serverRootDir = gitRootDir.resolve("server"); + Path otherRootDir = gitRootDir.resolve("other"); + sshd.setSubsystemFactories(Collections.singletonList(new SftpSubsystemFactory())); + sshd.setCommandFactory(new GitPackCommandFactory(GitLocationResolver.constantPath(serverRootDir)) + .withGitPackConfiguration(packConfig)); + sshd.start(); + + int port = sshd.getPort(); + try { + CommonTestSupportUtils.deleteRecursive(serverRootDir); + Files.createDirectory(serverRootDir); + String repoName = getCurrentTestName() + Constants.DOT_GIT_EXT; + Path otherDir = otherRootDir.resolve(repoName); + CommonTestSupportUtils.deleteRecursive(otherDir); + Git.init().setBare(true).setDirectory(otherDir.toFile()).call().close(); + + JSch.setConfig("StrictHostKeyChecking", "no"); + CredentialsProvider + .setDefault(new UsernamePasswordCredentialsProvider(getCurrentTestName(), getCurrentTestName())); + Path localRootDir = gitRootDir.resolve("local"); + Path localDir = localRootDir.resolve(repoName); + CommonTestSupportUtils.deleteRecursive(localDir); + + SshClient client = SshClient.setUpDefaultClient(); + SshSessionFactory.setInstance(new GitSshdSessionFactory(client)); + try { + assertThrows(Exception.class, () -> { + Git.cloneRepository().setURI("ssh://" + getCurrentTestName() + "@" + TEST_LOCALHOST + ":" + port + + "/foo/../../other/" + repoName) + .setDirectory(localDir.toFile()).call().close(); + }); + } finally { + client.stop(); + } + } finally { + sshd.stop(); + } + } finally { + SystemReader.setInstance(defaultSystemReader); + } + } + + @Test + void symLinkAllowed() throws Exception { + Assumptions.assumeFalse(OsUtils.isWin32(), "On windows this activates TortoisePlink, plus test uses symlinks"); + + Path gitRootDir = getTempTargetRelativeFile(getClass().getSimpleName()); + SystemReader defaultSystemReader = mockGitConfig(gitRootDir); + try (SshServer sshd = setupTestServer()) { + GitPackTestConfig packConfig = new GitPackTestConfig(); + Path serverRootDir = gitRootDir.resolve("server"); + Path otherRootDir = gitRootDir.resolve("tenants"); + sshd.setSubsystemFactories(Collections.singletonList(new SftpSubsystemFactory())); + sshd.setCommandFactory(new GitPackCommandFactory(GitLocationResolver.constantPath(serverRootDir)) + .withGitPackConfiguration(packConfig)); + sshd.start(); + + int port = sshd.getPort(); + try { + CommonTestSupportUtils.deleteRecursive(serverRootDir); + Files.createDirectory(serverRootDir); + CommonTestSupportUtils.deleteRecursive(otherRootDir); + Path realTenantDir = Files.createDirectories(otherRootDir.resolve("tenantX")); + String repoName = getCurrentTestName() + Constants.DOT_GIT_EXT; + Path otherDir = realTenantDir.resolve(repoName); + CommonTestSupportUtils.deleteRecursive(otherDir); + Git.init().setBare(true).setDirectory(otherDir.toFile()).call().close(); + Path symLinkPath = serverRootDir.resolve("Tenant-X"); + Files.createSymbolicLink(symLinkPath, realTenantDir); + + JSch.setConfig("StrictHostKeyChecking", "no"); + CredentialsProvider + .setDefault(new UsernamePasswordCredentialsProvider(getCurrentTestName(), getCurrentTestName())); + Path localRootDir = gitRootDir.resolve("local"); + Path localDir = localRootDir.resolve(repoName); + CommonTestSupportUtils.deleteRecursive(localDir); + + SshClient client = SshClient.setUpDefaultClient(); + SshSessionFactory.setInstance(new GitSshdSessionFactory(client)); + try (Git git = Git.cloneRepository() + .setURI("ssh://" + getCurrentTestName() + "@" + TEST_LOCALHOST + ":" + port + "/Tenant-X/" + repoName) + .setDirectory(localDir.toFile()).call()) { + assertTrue(localDir.resolve(Constants.DOT_GIT).toFile().isDirectory()); + } finally { + client.stop(); + } + } finally { + sshd.stop(); + } + } finally { + SystemReader.setInstance(defaultSystemReader); + } + } + + @Test + void gitRootAccessDenied() throws Exception { + Assumptions.assumeFalse(OsUtils.isWin32(), "On windows this activates TortoisePlink"); + + Path gitRootDir = getTempTargetRelativeFile(getClass().getSimpleName()); + SystemReader defaultSystemReader = mockGitConfig(gitRootDir); + try (SshServer sshd = setupTestServer()) { + GitPackTestConfig packConfig = new GitPackTestConfig(); + Path serverRootDir = gitRootDir.resolve("server"); + sshd.setSubsystemFactories(Collections.singletonList(new SftpSubsystemFactory())); + sshd.setCommandFactory(new GitPackCommandFactory(GitLocationResolver.constantPath(serverRootDir)) + .withGitPackConfiguration(packConfig)); + sshd.start(); + + int port = sshd.getPort(); + try { + CommonTestSupportUtils.deleteRecursive(serverRootDir); + Files.createDirectory(serverRootDir); + String repoName = getCurrentTestName() + Constants.DOT_GIT_EXT; + Git.init().setBare(true).setDirectory(serverRootDir.toFile()).call().close(); + + JSch.setConfig("StrictHostKeyChecking", "no"); + CredentialsProvider + .setDefault(new UsernamePasswordCredentialsProvider(getCurrentTestName(), getCurrentTestName())); + Path localRootDir = gitRootDir.resolve("local"); + Path localDir = localRootDir.resolve(repoName); + CommonTestSupportUtils.deleteRecursive(localDir); + + SshClient client = SshClient.setUpDefaultClient(); + SshSessionFactory.setInstance(new GitSshdSessionFactory(client)); + try { + assertThrows(Exception.class, () -> { + Git.cloneRepository().setURI( + "ssh://" + getCurrentTestName() + "@" + TEST_LOCALHOST + ":" + port + "/foo/../") + .setDirectory(localDir.toFile()).call().close(); + }); + } finally { + client.stop(); + } + } finally { + sshd.stop(); + } + } finally { + SystemReader.setInstance(defaultSystemReader); + } + } + + @Test + void giCannotEscapeRoot() throws Exception { + Assumptions.assumeFalse(OsUtils.isWin32(), "On windows this activates TortoisePlink"); + + Path gitRootDir = getTempTargetRelativeFile(getClass().getSimpleName()); + SystemReader defaultSystemReader = mockGitConfig(gitRootDir); + Git.init().setBare(true).setDirectory(gitRootDir.toFile()).call().close(); + try (SshServer sshd = setupTestServer()) { + GitPackTestConfig packConfig = new GitPackTestConfig(); + Path serverRootDir = gitRootDir.resolve("server"); + sshd.setSubsystemFactories(Collections.singletonList(new SftpSubsystemFactory())); + sshd.setCommandFactory(new GitPackCommandFactory(GitLocationResolver.constantPath(serverRootDir)) + .withGitPackConfiguration(packConfig)); + sshd.start(); + + int port = sshd.getPort(); + try { + CommonTestSupportUtils.deleteRecursive(serverRootDir); + Files.createDirectory(serverRootDir); + String repoName = getCurrentTestName() + Constants.DOT_GIT_EXT; + + JSch.setConfig("StrictHostKeyChecking", "no"); + CredentialsProvider + .setDefault(new UsernamePasswordCredentialsProvider(getCurrentTestName(), getCurrentTestName())); + Path localRootDir = gitRootDir.resolve("local"); + Path localDir = localRootDir.resolve(repoName); + CommonTestSupportUtils.deleteRecursive(localDir); + + SshClient client = SshClient.setUpDefaultClient(); + SshSessionFactory.setInstance(new GitSshdSessionFactory(client)); + try { + assertThrows(Exception.class, () -> { + Git.cloneRepository() + .setURI("ssh://" + getCurrentTestName() + "@" + TEST_LOCALHOST + ":" + port + "/./foo/../../") + .setDirectory(localDir.toFile()).call().close(); + }); + } finally { + client.stop(); + } + } finally { + sshd.stop(); + } + } finally { + SystemReader.setInstance(defaultSystemReader); + } + } + private static class GitPackTestConfig implements GitPackConfiguration { boolean receivePackCalled; boolean uploadPackCalled;
