This is an automated email from the ASF dual-hosted git repository. tomaswolf pushed a commit to branch rc-2.19.0 in repository https://gitbox.apache.org/repos/asf/mina-sshd.git
commit d42fb84244c784d3c3cc0bb84a1e92943f8a56cb Author: Thomas Wolf <[email protected]> AuthorDate: Wed Jun 10 07:58:30 2026 +0200 sshd-git: improve handling of repository paths Tighten the path checks: remove all leading separators, forbid colons on Windows, and double check before returning the resolved path. --- .../org/apache/sshd/git/AbstractGitCommand.java | 22 +++++++--- .../apache/sshd/git/AbstractGitCommandTest.java | 51 ++++++++++++++++++++++ .../apache/sshd/git/pack/GitPackCommandTest.java | 2 +- 3 files changed, 69 insertions(+), 6 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 2127e1c01..361598929 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,6 +19,7 @@ package org.apache.sshd.git; +import java.io.File; import java.io.IOException; import java.io.OutputStream; import java.nio.file.InvalidPathException; @@ -30,6 +31,7 @@ import java.util.Objects; import org.apache.sshd.common.channel.ChannelOutputStream; import org.apache.sshd.common.util.GenericUtils; +import org.apache.sshd.common.util.OsUtils; import org.apache.sshd.common.util.threads.CloseableExecutorService; import org.apache.sshd.server.command.AbstractFileSystemCommand; @@ -148,10 +150,17 @@ public abstract class AbstractGitCommand 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) == '/')) { + // Strip any leading separator since we use paths relative to root. + while ((len > 0) && (pathArg.charAt(0) == '/' || pathArg.charAt(0) == File.separatorChar)) { pathArg = pathArg.substring(1); + len--; } + if (OsUtils.isWin32() && pathArg.indexOf(':') >= 0) { + // Catches "C:/Something" or also "C:Something". Windows path name components may not contain colons. + // Windows drive specs are not allowed here. + throw new IOException("Invalid git repository path " + originalPath); + } + Path gitRepoPath; try { gitRepoPath = Paths.get(pathArg); @@ -161,7 +170,7 @@ public abstract class AbstractGitCommand if (gitRepoPath.isAbsolute()) { throw new IOException("Absolute git repository path not allowed: " + originalPath); } - // Remove "." and ".." components + // Remove "." and ".." components. gitRepoPath = gitRepoPath.normalize(); int componentCount = gitRepoPath.getNameCount(); // Deny access to the server root directory itself. @@ -175,7 +184,10 @@ public abstract class AbstractGitCommand throw new IOException("Invalid git repository path " + originalPath); } } - return serverRoot.resolve(gitRepoPath); - + Path result = serverRoot.resolve(gitRepoPath).normalize(); + if (!result.startsWith(serverRoot.normalize())) { + throw new IOException("Invalid git repository path " + originalPath); + } + return result; } } diff --git a/sshd-git/src/test/java/org/apache/sshd/git/AbstractGitCommandTest.java b/sshd-git/src/test/java/org/apache/sshd/git/AbstractGitCommandTest.java new file mode 100644 index 000000000..a7a658de7 --- /dev/null +++ b/sshd-git/src/test/java/org/apache/sshd/git/AbstractGitCommandTest.java @@ -0,0 +1,51 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.sshd.git; + +import java.io.IOException; +import java.nio.file.Path; + +import org.apache.sshd.common.util.OsUtils; +import org.junit.jupiter.api.Assumptions; +import org.junit.jupiter.api.Test; + +class AbstractGitCommandTest extends GitTestSupport { + + AbstractGitCommandTest() { + super(); + } + + @Test + void windowsRootRelativePathCannotEscapeConfiguredRoot() throws Exception { + Assumptions.assumeTrue(OsUtils.isWin32(), "Windows root-relative paths are required for this test"); + Path workDir = getTempTargetRelativeFile(getClass().getSimpleName(), getCurrentTestName()).toAbsolutePath(); + Path serverRoot = workDir.resolve("server-root"); + Path outsideRepo = workDir.resolve("outside").resolve("repo.git"); + String outside = outsideRepo.toString().replace('\\', '/'); + String driveRelativePayload = "/" + outside.substring(2); // Skip the drive + try { + Path resolved = AbstractGitCommand.resolveGitRepo(serverRoot, driveRelativePayload); + // Case (1) that would be OK: implementation under test strips all leading separators + assertTrue(resolved.normalize().startsWith(serverRoot.normalize()), + "resolved path unexpectedly outside configured root: " + resolved); + } catch (IOException e) { + // case (2) that would be OK: implementation under test throws an exception + } + } +} 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 b7686375a..568bcdb3a 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 @@ -344,7 +344,7 @@ public class GitPackCommandTest extends GitTestSupport { } @Test - void giCannotEscapeRoot() throws Exception { + void gitCannotEscapeRoot() throws Exception { Assumptions.assumeFalse(OsUtils.isWin32(), "On windows this activates TortoisePlink"); Path gitRootDir = getTempTargetRelativeFile(getClass().getSimpleName());
