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 ab3672f03161cd9f5f7d622bff0335e513c9994d Author: Thomas Wolf <[email protected]> AuthorDate: Thu Jun 18 18:25:33 2026 +0200 SCP: validate filenames SCP D or C commands should always have only simple filenames. --- .../org/apache/sshd/scp/common/ScpFileOpener.java | 31 +++++- .../helpers/LocalFileScpTargetStreamResolver.java | 10 +- .../helpers/ScpReceivePathTraversalTest.java | 122 +++++++++++++++++++++ 3 files changed, 158 insertions(+), 5 deletions(-) diff --git a/sshd-scp/src/main/java/org/apache/sshd/scp/common/ScpFileOpener.java b/sshd-scp/src/main/java/org/apache/sshd/scp/common/ScpFileOpener.java index b9c637ac9..82cb32465 100644 --- a/sshd-scp/src/main/java/org/apache/sshd/scp/common/ScpFileOpener.java +++ b/sshd-scp/src/main/java/org/apache/sshd/scp/common/ScpFileOpener.java @@ -47,6 +47,7 @@ import org.apache.sshd.common.util.OsUtils; import org.apache.sshd.common.util.SelectorUtils; import org.apache.sshd.common.util.io.DirectoryScanner; import org.apache.sshd.common.util.io.IoUtils; +import org.apache.sshd.scp.common.helpers.ScpAckInfo; import org.apache.sshd.scp.common.helpers.ScpTimestampCommandDetails; /** @@ -57,6 +58,29 @@ import org.apache.sshd.scp.common.helpers.ScpTimestampCommandDetails; */ public interface ScpFileOpener { + /** + * Determines whether a file name received in a C or D command is valid. + * + * @param fs the {@link FileSystem} used + * @param fileName to check + * @return the fileName, if it passes the checks + * @throws InvalidPathException if the path is not valid + */ + default String checkRemoteFileName(FileSystem fs, String fileName) { + if (fileName == null || fileName.isEmpty() || ".".equals(fileName) || "..".equals(fileName) + || fileName.indexOf('/') >= 0) { + throw new InvalidPathException(fileName, "Not a valid SCP fileName"); + } + if (OsUtils.isWin32() && (fileName.indexOf('\\') >= 0 || fileName.indexOf(':') >= 0)) { + throw new InvalidPathException(fileName, "Not a valid SCP fileName"); + } + Path p = fs.getPath(fileName); + if (p.isAbsolute() || !fileName.equals(p.getFileName().toString())) { + throw new InvalidPathException(fileName, "Not a valid SCP fileName"); + } + return fileName; + } + /** * Invoked when receiving a new file to via a directory command * @@ -82,8 +106,11 @@ public interface ScpFileOpener { Path file = null; if (status && Files.isDirectory(localPath, options)) { - String localName = name.replace('/', File.separatorChar); - file = localPath.resolve(localName); + try { + file = localPath.resolve(checkRemoteFileName(localPath.getFileSystem(), name)); + } catch (InvalidPathException e) { + throw new ScpException("Invalid directory name '" + name + "' received", e, ScpAckInfo.ERROR); + } } else if (!status) { Path parent = localPath.getParent(); diff --git a/sshd-scp/src/main/java/org/apache/sshd/scp/common/helpers/LocalFileScpTargetStreamResolver.java b/sshd-scp/src/main/java/org/apache/sshd/scp/common/helpers/LocalFileScpTargetStreamResolver.java index a501a68f9..c0265e4c0 100644 --- a/sshd-scp/src/main/java/org/apache/sshd/scp/common/helpers/LocalFileScpTargetStreamResolver.java +++ b/sshd-scp/src/main/java/org/apache/sshd/scp/common/helpers/LocalFileScpTargetStreamResolver.java @@ -19,12 +19,12 @@ package org.apache.sshd.scp.common.helpers; -import java.io.File; import java.io.IOException; import java.io.OutputStream; import java.io.StreamCorruptedException; import java.nio.file.AccessDeniedException; import java.nio.file.Files; +import java.nio.file.InvalidPathException; import java.nio.file.LinkOption; import java.nio.file.OpenOption; import java.nio.file.Path; @@ -37,6 +37,7 @@ import java.util.concurrent.TimeUnit; import org.apache.sshd.common.session.Session; import org.apache.sshd.common.util.io.IoUtils; import org.apache.sshd.common.util.logging.AbstractLoggingBean; +import org.apache.sshd.scp.common.ScpException; import org.apache.sshd.scp.common.ScpFileOpener; import org.apache.sshd.scp.common.ScpTargetStreamResolver; @@ -70,8 +71,11 @@ public class LocalFileScpTargetStreamResolver extends AbstractLoggingBean implem LinkOption[] linkOptions = IoUtils.getLinkOptions(true); if (status && Files.isDirectory(path, linkOptions)) { - String localName = name.replace('/', File.separatorChar); // in case we are running on Windows - file = path.resolve(localName); + try { + file = path.resolve(opener.checkRemoteFileName(path.getFileSystem(), name)); + } catch (InvalidPathException e) { + throw new ScpException("Invalid file name '" + name + "' received", e, ScpAckInfo.ERROR); + } } else if (status && Files.isRegularFile(path, linkOptions)) { file = path; } else if (!status) { diff --git a/sshd-scp/src/test/java/org/apache/sshd/scp/common/helpers/ScpReceivePathTraversalTest.java b/sshd-scp/src/test/java/org/apache/sshd/scp/common/helpers/ScpReceivePathTraversalTest.java new file mode 100644 index 000000000..32a83c718 --- /dev/null +++ b/sshd-scp/src/test/java/org/apache/sshd/scp/common/helpers/ScpReceivePathTraversalTest.java @@ -0,0 +1,122 @@ +/* + * 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.scp.common.helpers; + +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.OutputStream; +import java.nio.charset.StandardCharsets; +import java.nio.file.FileSystems; +import java.nio.file.Files; +import java.nio.file.OpenOption; +import java.nio.file.Path; +import java.nio.file.StandardOpenOption; +import java.nio.file.attribute.PosixFilePermission; +import java.util.Set; + +import org.apache.sshd.common.session.Session; +import org.apache.sshd.scp.common.ScpHelper; +import org.apache.sshd.scp.common.ScpTransferEventListener; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; +import org.mockito.Mockito; + +import static org.junit.jupiter.api.Assertions.assertArrayEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; + +class ScpReceivePathTraversalTest { + + @TempDir + Path tempDir; + + @Test + void receiveFileNameCannotEscapeRequestedDirectory() throws Exception { + Path requestedDirectory = Files.createDirectories(tempDir.resolve("requested")); + Path outsideFile = requestedDirectory.resolve("..").resolve("outside.txt").normalize(); + byte[] payload = "written outside requested directory".getBytes(StandardCharsets.UTF_8); + ByteArrayInputStream input = new ByteArrayInputStream(scpFileReceiveScript("../outside.txt", payload)); + ByteArrayOutputStream output = new ByteArrayOutputStream(); + ScpHelper helper = new ScpHelper(Mockito.mock(Session.class), input, StandardCharsets.UTF_8, output, + StandardCharsets.UTF_8, FileSystems.getDefault(), new NonSyncScpFileOpener(), ScpTransferEventListener.EMPTY); + assertThrows(IOException.class, () -> helper.receive("scp -t " + requestedDirectory, requestedDirectory, false, false, + false, ScpHelper.MIN_RECEIVE_BUFFER_SIZE)); + assertFalse(Files.exists(outsideFile), "remote SCP name should not escape the requested directory"); + assertFalse(Files.exists(requestedDirectory.resolve("outside.txt"))); + } + + @Test + void receiveFileNameCannotOverwriteExistingFileOutsideRequestedDirectory() throws Exception { + Path requestedDirectory = Files.createDirectories(tempDir.resolve("requested")); + Path simulatedAutoloadedConfig = requestedDirectory.resolve("..").resolve("simulated-autoloaded-config.txt") + .normalize(); + byte[] originalData = "original safe config".getBytes(StandardCharsets.UTF_8); + Files.write(simulatedAutoloadedConfig, originalData); + byte[] payload = "attacker-controlled benign marker".getBytes(StandardCharsets.UTF_8); + ByteArrayInputStream input = new ByteArrayInputStream( + scpFileReceiveScript("../simulated-autoloaded-config.txt", payload)); + ByteArrayOutputStream output = new ByteArrayOutputStream(); + ScpHelper helper = new ScpHelper(Mockito.mock(Session.class), input, StandardCharsets.UTF_8, output, + StandardCharsets.UTF_8, FileSystems.getDefault(), new NonSyncScpFileOpener(), ScpTransferEventListener.EMPTY); + assertThrows(IOException.class, () -> helper.receive("scp -t " + requestedDirectory, requestedDirectory, false, false, + false, ScpHelper.MIN_RECEIVE_BUFFER_SIZE)); + assertArrayEquals(originalData, Files.readAllBytes(simulatedAutoloadedConfig)); + assertFalse(Files.exists(requestedDirectory.resolve("simulated-autoloaded-config.txt"))); + } + + @Test + void receiveDirectoryNameCannotEscapeRequestedDirectory() throws Exception { + Path requestedDirectory = Files.createDirectories(tempDir.resolve("requested")); + Path outsideDirectory = requestedDirectory.resolve("..").resolve("outside-dir").normalize(); + ByteArrayInputStream input = new ByteArrayInputStream(scpDirectoryReceiveScript("../outside-dir")); + ByteArrayOutputStream output = new ByteArrayOutputStream(); + ScpHelper helper = new ScpHelper(Mockito.mock(Session.class), input, StandardCharsets.UTF_8, output, + StandardCharsets.UTF_8, FileSystems.getDefault(), new NonSyncScpFileOpener(), ScpTransferEventListener.EMPTY); + assertThrows(IOException.class, () -> helper.receive("scp -rt " + requestedDirectory, requestedDirectory, true, false, + false, ScpHelper.MIN_RECEIVE_BUFFER_SIZE)); + assertFalse(Files.isDirectory(outsideDirectory), "remote SCP directory name should not escape the requested directory"); + assertFalse(Files.exists(requestedDirectory.resolve("outside-dir"))); + } + + private static byte[] scpFileReceiveScript(String name, byte[] payload) throws IOException { + ByteArrayOutputStream script = new ByteArrayOutputStream(); + script.write(("C0644 " + payload.length + " " + name + "\n").getBytes(StandardCharsets.UTF_8)); + script.write(payload); + script.write(ScpAckInfo.OK); + return script.toByteArray(); + } + + private static byte[] scpDirectoryReceiveScript(String name) throws IOException { + ByteArrayOutputStream script = new ByteArrayOutputStream(); + script.write(("D0755 0 " + name + "\n").getBytes(StandardCharsets.UTF_8)); + script.write((ScpDirEndCommandDetails.HEADER + "\n").getBytes(StandardCharsets.UTF_8)); + return script.toByteArray(); + } + + private static final class NonSyncScpFileOpener extends DefaultScpFileOpener { + @Override + public OutputStream openWrite( + Session session, Path file, long size, Set<PosixFilePermission> permissions, + OpenOption... options) throws IOException { + return Files.newOutputStream(file, StandardOpenOption.CREATE, StandardOpenOption.TRUNCATE_EXISTING, + StandardOpenOption.WRITE); + } + } +}
