diff --git a/CVE-2023-35887.patch b/CVE-2023-35887.patch new file mode 100644 index 0000000..9788882 --- /dev/null +++ b/CVE-2023-35887.patch @@ -0,0 +1,2338 @@ +From c20739b43aab0f7bf2ccad982a6cb37b9d5a8a0b Mon Sep 17 00:00:00 2001 +From: Guillaume Nodet +Date: Tue, 9 May 2023 15:17:26 +0200 +Subject: [PATCH] [SSHD-1234] Rooted file system can leak informations + +--- + .../file/root/RootedDirectoryStream.java | 63 +++ + .../common/file/root/RootedFileSystem.java | 5 + + .../file/root/RootedFileSystemProvider.java | 327 ++++++++---- + .../file/root/RootedFileSystemUtils.java | 55 ++ + .../root/RootedSecureDirectoryStream.java | 92 ++++ + .../sshd/common/file/util/BaseFileSystem.java | 23 +- + .../apache/sshd/common/util/io/IoUtils.java | 160 +++++- + .../root/RootedFileSystemProviderTest.java | 469 ++++++++++++++---- + .../sshd/common/util/io/IoUtilsTest.java | 48 ++ + .../util/test/CommonTestSupportUtils.java | 4 +- + .../client/fs/SftpFileSystemProvider.java | 4 +- + .../server/AbstractSftpSubsystemHelper.java | 101 +++- + .../sftp/server/SftpFileSystemAccessor.java | 94 +++- + .../sshd/sftp/server/SftpSubsystem.java | 8 +- + .../org/apache/sshd/sftp/client/SftpTest.java | 7 +- + .../sshd/sftp/client/SftpVersionsTest.java | 12 +- + 16 files changed, 1203 insertions(+), 269 deletions(-) + create mode 100644 sshd-common/src/main/java/org/apache/sshd/common/file/root/RootedDirectoryStream.java + create mode 100644 sshd-common/src/main/java/org/apache/sshd/common/file/root/RootedFileSystemUtils.java + create mode 100644 sshd-common/src/main/java/org/apache/sshd/common/file/root/RootedSecureDirectoryStream.java + +diff --git a/sshd-common/src/main/java/org/apache/sshd/common/file/root/RootedDirectoryStream.java b/sshd-common/src/main/java/org/apache/sshd/common/file/root/RootedDirectoryStream.java +new file mode 100644 +index 000000000..dde9e748d +--- /dev/null ++++ b/sshd-common/src/main/java/org/apache/sshd/common/file/root/RootedDirectoryStream.java +@@ -0,0 +1,63 @@ ++/* ++ * 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.common.file.root; ++ ++import java.io.IOException; ++import java.nio.file.DirectoryStream; ++import java.nio.file.Path; ++import java.util.Iterator; ++ ++/** ++ * secure directory stream proxy for a {@link RootedFileSystem} ++ * ++ * @author Apache MINA SSHD Project ++ */ ++public class RootedDirectoryStream implements DirectoryStream { ++ protected final RootedFileSystem rfs; ++ protected final DirectoryStream delegate; ++ ++ public RootedDirectoryStream(RootedFileSystem rfs, DirectoryStream delegate) { ++ this.rfs = rfs; ++ this.delegate = delegate; ++ } ++ ++ @Override ++ public Iterator iterator() { ++ return root(rfs, delegate.iterator()); ++ } ++ ++ @Override ++ public void close() throws IOException { ++ delegate.close(); ++ } ++ ++ protected Iterator root(RootedFileSystem rfs, Iterator iter) { ++ return new Iterator() { ++ @Override ++ public boolean hasNext() { ++ return iter.hasNext(); ++ } ++ ++ @Override ++ public Path next() { ++ return rfs.provider().root(rfs, iter.next()); ++ } ++ }; ++ } ++} +diff --git a/sshd-common/src/main/java/org/apache/sshd/common/file/root/RootedFileSystem.java b/sshd-common/src/main/java/org/apache/sshd/common/file/root/RootedFileSystem.java +index 1b9a71ca1..efd0b134d 100644 +--- a/sshd-common/src/main/java/org/apache/sshd/common/file/root/RootedFileSystem.java ++++ b/sshd-common/src/main/java/org/apache/sshd/common/file/root/RootedFileSystem.java +@@ -94,6 +94,11 @@ public Iterable getFileStores() { + return rootFs.getFileStores(); + } + ++ @Override ++ protected boolean hostFsHasWindowsSeparator() { ++ return "\\".equals(getRoot().getFileSystem().getSeparator()); ++ } ++ + @Override + public String toString() { + return rootPath.toString(); +diff --git a/sshd-common/src/main/java/org/apache/sshd/common/file/root/RootedFileSystemProvider.java b/sshd-common/src/main/java/org/apache/sshd/common/file/root/RootedFileSystemProvider.java +index 0855c7751..9922582b1 100644 +--- a/sshd-common/src/main/java/org/apache/sshd/common/file/root/RootedFileSystemProvider.java ++++ b/sshd-common/src/main/java/org/apache/sshd/common/file/root/RootedFileSystemProvider.java +@@ -18,6 +18,7 @@ + */ + package org.apache.sshd.common.file.root; + ++import java.io.FileNotFoundException; + import java.io.IOException; + import java.io.InputStream; + import java.io.OutputStream; +@@ -26,32 +27,40 @@ + import java.nio.channels.AsynchronousFileChannel; + import java.nio.channels.FileChannel; + import java.nio.channels.SeekableByteChannel; ++import java.nio.file.AccessDeniedException; + import java.nio.file.AccessMode; ++import java.nio.file.AtomicMoveNotSupportedException; + import java.nio.file.CopyOption; ++import java.nio.file.DirectoryNotEmptyException; + import java.nio.file.DirectoryStream; ++import java.nio.file.FileAlreadyExistsException; + import java.nio.file.FileStore; + import java.nio.file.FileSystem; + import java.nio.file.FileSystemAlreadyExistsException; ++import java.nio.file.FileSystemException; ++import java.nio.file.FileSystemLoopException; + import java.nio.file.FileSystemNotFoundException; + import java.nio.file.Files; + import java.nio.file.InvalidPathException; + import java.nio.file.LinkOption; ++import java.nio.file.NoSuchFileException; ++import java.nio.file.NotDirectoryException; ++import java.nio.file.NotLinkException; + import java.nio.file.OpenOption; + import java.nio.file.Path; + import java.nio.file.Paths; + import java.nio.file.ProviderMismatchException; ++import java.nio.file.SecureDirectoryStream; + import java.nio.file.attribute.BasicFileAttributes; + import java.nio.file.attribute.FileAttribute; + import java.nio.file.attribute.FileAttributeView; + import java.nio.file.spi.FileSystemProvider; + import java.util.HashMap; +-import java.util.Iterator; + import java.util.Map; + import java.util.Objects; + import java.util.Set; + import java.util.concurrent.ExecutorService; + +-import org.apache.sshd.common.util.ValidateUtils; + import org.apache.sshd.common.util.io.IoUtils; + import org.slf4j.Logger; + import org.slf4j.LoggerFactory; +@@ -157,14 +166,22 @@ public Path getPath(URI uri) { + public InputStream newInputStream(Path path, OpenOption... options) throws IOException { + Path r = unroot(path); + FileSystemProvider p = provider(r); +- return p.newInputStream(r, options); ++ try { ++ return p.newInputStream(r, options); ++ } catch (IOException ex) { ++ throw translateIoException(ex, path); ++ } + } + + @Override + public OutputStream newOutputStream(Path path, OpenOption... options) throws IOException { + Path r = unroot(path); + FileSystemProvider p = provider(r); +- return p.newOutputStream(r, options); ++ try { ++ return p.newOutputStream(r, options); ++ } catch (IOException ex) { ++ throw translateIoException(ex, path); ++ } + } + + @Override +@@ -172,7 +189,11 @@ public FileChannel newFileChannel(Path path, Set options, + throws IOException { + Path r = unroot(path); + FileSystemProvider p = provider(r); +- return p.newFileChannel(r, options, attrs); ++ try { ++ return p.newFileChannel(r, options, attrs); ++ } catch (IOException ex) { ++ throw translateIoException(ex, path); ++ } + } + + @Override +@@ -181,7 +202,11 @@ public AsynchronousFileChannel newAsynchronousFileChannel( + throws IOException { + Path r = unroot(path); + FileSystemProvider p = provider(r); +- return p.newAsynchronousFileChannel(r, options, executor, attrs); ++ try { ++ return p.newAsynchronousFileChannel(r, options, executor, attrs); ++ } catch (IOException ex) { ++ throw translateIoException(ex, path); ++ } + } + + @Override +@@ -189,82 +214,74 @@ public SeekableByteChannel newByteChannel(Path path, Set o + throws IOException { + Path r = unroot(path); + FileSystemProvider p = provider(r); +- return p.newByteChannel(r, options, attrs); ++ try { ++ return p.newByteChannel(r, options, attrs); ++ } catch (IOException ex) { ++ throw translateIoException(ex, path); ++ } + } + + @Override + public DirectoryStream newDirectoryStream(Path dir, DirectoryStream.Filter filter) throws IOException { + Path r = unroot(dir); + FileSystemProvider p = provider(r); +- return root(((RootedPath) dir).getFileSystem(), p.newDirectoryStream(r, filter)); ++ try { ++ return root(((RootedPath) dir).getFileSystem(), p.newDirectoryStream(r, filter)); ++ } catch (IOException ex) { ++ throw translateIoException(ex, dir); ++ } + } + + protected DirectoryStream root(RootedFileSystem rfs, DirectoryStream ds) { +- return new DirectoryStream() { +- @Override +- public Iterator iterator() { +- return root(rfs, ds.iterator()); +- } +- +- @Override +- public void close() throws IOException { +- ds.close(); +- } +- }; +- } +- +- protected Iterator root(RootedFileSystem rfs, Iterator iter) { +- return new Iterator() { +- @Override +- public boolean hasNext() { +- return iter.hasNext(); +- } +- +- @Override +- public Path next() { +- return root(rfs, iter.next()); +- } +- }; ++ if (ds instanceof SecureDirectoryStream) { ++ return new RootedSecureDirectoryStream(rfs, (SecureDirectoryStream) ds); ++ } ++ return new RootedDirectoryStream(rfs, ds); + } + + @Override + public void createDirectory(Path dir, FileAttribute... attrs) throws IOException { + Path r = unroot(dir); + FileSystemProvider p = provider(r); +- p.createDirectory(r, attrs); ++ try { ++ p.createDirectory(r, attrs); ++ } catch (IOException ex) { ++ throw translateIoException(ex, dir); ++ } + } + + @Override + public void createSymbolicLink(Path link, Path target, FileAttribute... attrs) throws IOException { +- createLink(link, target, true, attrs); +- } +- +- @Override +- public void createLink(Path link, Path existing) throws IOException { +- createLink(link, existing, false); +- } +- +- protected void createLink(Path link, Path target, boolean symLink, FileAttribute... attrs) throws IOException { ++ // make sure symlink cannot break out of chroot jail. If it is unsafe, simply thrown an exception. This is ++ // to ensure that symlink semantics are maintained when it is safe, and creation fails when not. ++ RootedFileSystemUtils.validateSafeRelativeSymlink(target); + Path l = unroot(link); +- Path t = unroot(target); +- /* +- * For a symbolic link preserve the relative path +- */ +- if (symLink && (!target.isAbsolute())) { +- RootedFileSystem rfs = ((RootedPath) target).getFileSystem(); +- Path root = rfs.getRoot(); +- t = root.relativize(t); +- } ++ Path t = target.isAbsolute() ? unroot(target) : l.getFileSystem().getPath(target.toString()); + + FileSystemProvider p = provider(l); +- if (symLink) { ++ try { + p.createSymbolicLink(l, t, attrs); +- } else { +- p.createLink(l, t); ++ ++ if (log.isDebugEnabled()) { ++ log.debug("createSymbolicLink({} => {}", l, t); ++ } ++ } catch (IOException ex) { ++ throw translateIoException(ex, link); + } ++ } + +- if (log.isDebugEnabled()) { +- log.debug("createLink(symbolic={}) {} => {}", symLink, l, t); ++ @Override ++ public void createLink(Path link, Path existing) throws IOException { ++ Path l = unroot(link); ++ Path t = unroot(existing); ++ ++ try { ++ provider(l).createLink(l, t); ++ if (log.isDebugEnabled()) { ++ log.debug("createLink({} => {}", l, t); ++ } ++ } catch (IOException ex) { ++ throw translateIoException(ex, link); + } + } + +@@ -275,7 +292,11 @@ public void delete(Path path) throws IOException { + log.trace("delete({}): {}", path, r); + } + FileSystemProvider p = provider(r); +- p.delete(r); ++ try { ++ p.delete(r); ++ } catch (IOException ex) { ++ throw translateIoException(ex, path); ++ } + } + + @Override +@@ -285,19 +306,27 @@ public boolean deleteIfExists(Path path) throws IOException { + log.trace("deleteIfExists({}): {}", path, r); + } + FileSystemProvider p = provider(r); +- return p.deleteIfExists(r); ++ try { ++ return p.deleteIfExists(r); ++ } catch (IOException ex) { ++ throw translateIoException(ex, path); ++ } + } + + @Override + public Path readSymbolicLink(Path link) throws IOException { + Path r = unroot(link); + FileSystemProvider p = provider(r); +- Path t = p.readSymbolicLink(r); +- Path target = root((RootedFileSystem) link.getFileSystem(), t); +- if (log.isTraceEnabled()) { +- log.trace("readSymbolicLink({})[{}]: {}[{}]", link, r, target, t); ++ try { ++ Path t = p.readSymbolicLink(r); ++ Path target = root((RootedFileSystem) link.getFileSystem(), t); ++ if (log.isTraceEnabled()) { ++ log.trace("readSymbolicLink({})[{}]: {}[{}]", link, r, target, t); ++ } ++ return target; ++ } catch (IOException ex) { ++ throw translateIoException(ex, link); + } +- return target; + } + + @Override +@@ -308,7 +337,11 @@ public void copy(Path source, Path target, CopyOption... options) throws IOExcep + log.trace("copy({})[{}]: {}[{}]", source, s, target, t); + } + FileSystemProvider p = provider(s); +- p.copy(s, t, options); ++ try { ++ p.copy(s, t, options); ++ } catch (IOException ex) { ++ throw translateIoException(ex, source); ++ } + } + + @Override +@@ -319,7 +352,11 @@ public void move(Path source, Path target, CopyOption... options) throws IOExcep + log.trace("move({})[{}]: {}[{}]", source, s, target, t); + } + FileSystemProvider p = provider(s); +- p.move(s, t, options); ++ try { ++ p.move(s, t, options); ++ } catch (IOException ex) { ++ throw translateIoException(ex, source); ++ } + } + + @Override +@@ -327,21 +364,33 @@ public boolean isSameFile(Path path, Path path2) throws IOException { + Path r = unroot(path); + Path r2 = unroot(path2); + FileSystemProvider p = provider(r); +- return p.isSameFile(r, r2); ++ try { ++ return p.isSameFile(r, r2); ++ } catch (IOException ex) { ++ throw translateIoException(ex, path); ++ } + } + + @Override + public boolean isHidden(Path path) throws IOException { + Path r = unroot(path); + FileSystemProvider p = provider(r); +- return p.isHidden(r); ++ try { ++ return p.isHidden(r); ++ } catch (IOException ex) { ++ throw translateIoException(ex, path); ++ } + } + + @Override + public FileStore getFileStore(Path path) throws IOException { + RootedFileSystem fileSystem = getFileSystem(path); + Path root = fileSystem.getRoot(); +- return Files.getFileStore(root); ++ try { ++ return Files.getFileStore(root); ++ } catch (IOException ex) { ++ throw translateIoException(ex, path); ++ } + } + + protected RootedFileSystem getFileSystem(Path path) throws FileSystemNotFoundException { +@@ -384,7 +433,11 @@ protected RootedFileSystem getFileSystem(Path path) throws FileSystemNotFoundExc + public void checkAccess(Path path, AccessMode... modes) throws IOException { + Path r = unroot(path); + FileSystemProvider p = provider(r); +- p.checkAccess(r, modes); ++ try { ++ p.checkAccess(r, modes); ++ } catch (IOException ex) { ++ throw translateIoException(ex, path); ++ } + } + + @Override +@@ -403,18 +456,26 @@ public A readAttributes(Path path, Class type + } + + FileSystemProvider p = provider(r); +- return p.readAttributes(r, type, options); ++ try { ++ return p.readAttributes(r, type, options); ++ } catch (IOException ex) { ++ throw translateIoException(ex, path); ++ } + } + + @Override + public Map readAttributes(Path path, String attributes, LinkOption... options) throws IOException { + Path r = unroot(path); + FileSystemProvider p = provider(r); +- Map attrs = p.readAttributes(r, attributes, options); +- if (log.isTraceEnabled()) { +- log.trace("readAttributes({})[{}] {}: {}", path, r, attributes, attrs); ++ try { ++ Map attrs = p.readAttributes(r, attributes, options); ++ if (log.isTraceEnabled()) { ++ log.trace("readAttributes({})[{}] {}: {}", path, r, attributes, attrs); ++ } ++ return attrs; ++ } catch (IOException ex) { ++ throw translateIoException(ex, path); + } +- return attrs; + } + + @Override +@@ -424,7 +485,11 @@ public void setAttribute(Path path, String attribute, Object value, LinkOption.. + log.trace("setAttribute({})[{}] {}={}", path, r, attribute, value); + } + FileSystemProvider p = provider(r); +- p.setAttribute(r, attribute, value, options); ++ try { ++ p.setAttribute(r, attribute, value, options); ++ } catch (IOException ex) { ++ throw translateIoException(ex, path); ++ } + } + + protected FileSystemProvider provider(Path path) { +@@ -434,10 +499,33 @@ protected FileSystemProvider provider(Path path) { + + protected Path root(RootedFileSystem rfs, Path nat) { + if (nat.isAbsolute()) { ++ // preferred case - this isn't a symlink out of our jail ++ if (nat.startsWith(rfs.getRoot())) { ++ // If we have the same number of parts as the root, and start with the root, we must be the root. ++ if (nat.getNameCount() == rfs.getRoot().getNameCount()) { ++ return rfs.getPath("/"); ++ } ++ ++ // We are the root, and more. Get the first name past the root because of how getPath works ++ String firstName = "/" + nat.getName(rfs.getRoot().getNameCount()); ++ ++ // the rooted path should have the number of parts past the root ++ String[] varargs = new String[nat.getNameCount() - rfs.getRoot().getNameCount() - 1]; ++ int varargsCounter = 0; ++ for (int i = 1 + rfs.getRoot().getNameCount(); i < nat.getNameCount(); i++) { ++ varargs[varargsCounter++] = nat.getName(i).toString(); ++ } ++ return rfs.getPath(firstName, varargs); ++ } ++ ++ // This is the case where there's a symlink jailbreak, so we return a relative link as the directories above ++ // the chroot don't make sense to present ++ // The behavior with the fs class is that we follow the symlink. Note that this is dangerous. + Path root = rfs.getRoot(); + Path rel = root.relativize(nat); +- return rfs.getPath("/" + rel.toString()); ++ return rfs.getPath("/" + rel); + } else { ++ // For a relative symlink, simply return it as a RootedPath. Note that this may break out of the chroot. + return rfs.getPath(nat.toString()); + } + } +@@ -466,39 +554,74 @@ protected Path unroot(Path path) { + * @throws InvalidPathException If the resolved path is not a proper sub-path of the rooted file system + */ + protected Path resolveLocalPath(RootedPath path) { +- RootedPath absPath = Objects.requireNonNull(path, "No rooted path to resolve").toAbsolutePath(); +- RootedFileSystem rfs = absPath.getFileSystem(); ++ Objects.requireNonNull(path, "No rooted path to resolve"); ++ RootedFileSystem rfs = path.getFileSystem(); + Path root = rfs.getRoot(); +- FileSystem lfs = root.getFileSystem(); ++ // initialize a list for the new file name parts ++ Path resolved = IoUtils.chroot(root, path); + +- String rSep = ValidateUtils.checkNotNullAndNotEmpty(rfs.getSeparator(), "No rooted file system separator"); +- ValidateUtils.checkTrue(rSep.length() == 1, "Bad rooted file system separator: %s", rSep); +- char rootedSeparator = rSep.charAt(0); ++ /* ++ * This can happen for Windows since we represent its paths as /C:/some/path, so substring(1) yields ++ * C:/some/path - which is resolved as an absolute path (which we don't want). ++ * ++ * This also is a security assertion to protect against unknown attempts to break out of the chroot jail ++ */ ++ if (!resolved.normalize().startsWith(root)) { ++ throw new InvalidPathException(root.toString(), "Not under root"); ++ } ++ return resolved; ++ } + +- String lSep = ValidateUtils.checkNotNullAndNotEmpty(lfs.getSeparator(), "No local file system separator"); +- ValidateUtils.checkTrue(lSep.length() == 1, "Bad local file system separator: %s", lSep); +- char localSeparator = lSep.charAt(0); ++ private IOException translateIoException(IOException ex, Path rootedPath) { ++ // cast is safe as path was unrooted earlier. ++ RootedPath rootedPathCasted = (RootedPath) rootedPath; ++ Path root = rootedPathCasted.getFileSystem().getRoot(); ++ ++ if (ex instanceof FileSystemException) { ++ String file = fixExceptionFileName(root, rootedPath, ((FileSystemException) ex).getFile()); ++ String otherFile = fixExceptionFileName(root, rootedPath, ((FileSystemException) ex).getOtherFile()); ++ String reason = ((FileSystemException) ex).getReason(); ++ if (NoSuchFileException.class.equals(ex.getClass())) { ++ return new NoSuchFileException(file, otherFile, reason); ++ } else if (FileSystemLoopException.class.equals(ex.getClass())) { ++ return new FileSystemLoopException(file); ++ } else if (NotDirectoryException.class.equals(ex.getClass())) { ++ return new NotDirectoryException(file); ++ } else if (DirectoryNotEmptyException.class.equals(ex.getClass())) { ++ return new DirectoryNotEmptyException(file); ++ } else if (NotLinkException.class.equals(ex.getClass())) { ++ return new NotLinkException(file); ++ } else if (AtomicMoveNotSupportedException.class.equals(ex.getClass())) { ++ return new AtomicMoveNotSupportedException(file, otherFile, reason); ++ } else if (FileAlreadyExistsException.class.equals(ex.getClass())) { ++ return new FileAlreadyExistsException(file, otherFile, reason); ++ } else if (AccessDeniedException.class.equals(ex.getClass())) { ++ return new AccessDeniedException(file, otherFile, reason); ++ } ++ return new FileSystemException(file, otherFile, reason); ++ } else if (ex.getClass().equals(FileNotFoundException.class)) { ++ return new FileNotFoundException(ex.getLocalizedMessage().replace(root.toString(), "")); ++ } ++ // not sure how to translate, so leave as is. Hopefully does not leak data ++ return ex; ++ } + +- String r = absPath.toString(); +- String subPath = r.substring(1); +- if (rootedSeparator != localSeparator) { +- subPath = subPath.replace(rootedSeparator, localSeparator); ++ private String fixExceptionFileName(Path root, Path rootedPath, String fileName) { ++ if (fileName == null) { ++ return null; + } + +- Path resolved = root.resolve(subPath); +- resolved = resolved.normalize(); +- resolved = resolved.toAbsolutePath(); +- if (log.isTraceEnabled()) { +- log.trace("resolveLocalPath({}): {}", absPath, resolved); ++ Path toFix = root.getFileSystem().getPath(fileName); ++ if (toFix.getNameCount() == root.getNameCount()) { ++ // return the root ++ return rootedPath.getFileSystem().getSeparator(); + } + +- /* +- * This can happen for Windows since we represent its paths as /C:/some/path, so substring(1) yields +- * C:/some/path - which is resolved as an absolute path (which we don't want). +- */ +- if (!resolved.startsWith(root)) { +- throw new InvalidPathException(r, "Not under root"); ++ StringBuilder ret = new StringBuilder(); ++ for (int partNum = root.getNameCount(); partNum < toFix.getNameCount(); partNum++) { ++ ret.append(rootedPath.getFileSystem().getSeparator()); ++ ret.append(toFix.getName(partNum++)); + } +- return resolved; ++ return ret.toString(); + } + } +diff --git a/sshd-common/src/main/java/org/apache/sshd/common/file/root/RootedFileSystemUtils.java b/sshd-common/src/main/java/org/apache/sshd/common/file/root/RootedFileSystemUtils.java +new file mode 100644 +index 000000000..6b0b164e9 +--- /dev/null ++++ b/sshd-common/src/main/java/org/apache/sshd/common/file/root/RootedFileSystemUtils.java +@@ -0,0 +1,55 @@ ++/* ++ * 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.common.file.root; ++ ++import java.nio.file.InvalidPathException; ++import java.nio.file.Path; ++ ++/** ++ * Utility functions for rooted file utils ++ */ ++public final class RootedFileSystemUtils { ++ ++ private RootedFileSystemUtils() { ++ // do not construct ++ } ++ ++ /** ++ * Validate that the relative path target is safe. This means that at no point in the path can there be more ".." ++ * than path parts. ++ * ++ * @param target the target directory to validate is safe. ++ */ ++ public static void validateSafeRelativeSymlink(Path target) { ++ int numNames = 0; ++ int numCdUps = 0; ++ for (int i = 0; i < target.getNameCount(); i++) { ++ if ("..".equals(target.getName(i).toString())) { ++ numCdUps++; ++ } else if (!".".equals(target.getName(i).toString())) { ++ numNames++; ++ } ++ ++ // need to check at each part to prevent data leakage outside of chroot ++ if (numCdUps > numNames) { ++ throw new InvalidPathException(target.toString(), "Symlink would exit chroot: " + target); ++ } ++ } ++ } ++} +diff --git a/sshd-common/src/main/java/org/apache/sshd/common/file/root/RootedSecureDirectoryStream.java b/sshd-common/src/main/java/org/apache/sshd/common/file/root/RootedSecureDirectoryStream.java +new file mode 100644 +index 000000000..6fe811c5b +--- /dev/null ++++ b/sshd-common/src/main/java/org/apache/sshd/common/file/root/RootedSecureDirectoryStream.java +@@ -0,0 +1,92 @@ ++/* ++ * 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.common.file.root; ++ ++import java.io.IOException; ++import java.nio.channels.SeekableByteChannel; ++import java.nio.file.LinkOption; ++import java.nio.file.OpenOption; ++import java.nio.file.Path; ++import java.nio.file.SecureDirectoryStream; ++import java.nio.file.attribute.FileAttribute; ++import java.nio.file.attribute.FileAttributeView; ++import java.util.Set; ++ ++/** ++ * A secure directory stream proxy for a {@link RootedFileSystem} ++ * ++ * @author Apache MINA SSHD Project ++ */ ++public class RootedSecureDirectoryStream extends RootedDirectoryStream implements SecureDirectoryStream { ++ ++ public RootedSecureDirectoryStream(RootedFileSystem rfs, SecureDirectoryStream delegate) { ++ super(rfs, delegate); ++ } ++ ++ @Override ++ public SecureDirectoryStream newDirectoryStream(Path path, LinkOption... options) throws IOException { ++ return new RootedSecureDirectoryStream(rfs, delegate().newDirectoryStream(fixPath(path), options)); ++ } ++ ++ protected Path fixPath(Path p) { ++ if (p.isAbsolute()) { ++ return rfs.provider().unroot(p); ++ } ++ ++ // convert to root fs path. ++ // Note: this IS able to go below the root directory by design - a way to break out of chroot. ++ // Be very cautious using this. ++ return rfs.getRootFileSystem().getPath(p.toString()); ++ } ++ ++ @Override ++ public SeekableByteChannel newByteChannel(Path path, Set options, FileAttribute... attrs) ++ throws IOException { ++ return delegate().newByteChannel(fixPath(path), options, attrs); ++ } ++ ++ @Override ++ public void deleteFile(Path path) throws IOException { ++ delegate().deleteFile(fixPath(path)); ++ } ++ ++ @Override ++ public void deleteDirectory(Path path) throws IOException { ++ delegate().deleteDirectory(fixPath(path)); ++ } ++ ++ @Override ++ public void move(Path srcpath, SecureDirectoryStream targetdir, Path targetpath) throws IOException { ++ delegate().move(fixPath(srcpath), targetdir, targetpath); ++ } ++ ++ @Override ++ public V getFileAttributeView(Class type) { ++ return delegate().getFileAttributeView(type); ++ } ++ ++ @Override ++ public V getFileAttributeView(Path path, Class type, LinkOption... options) { ++ return delegate().getFileAttributeView(path, type, options); ++ } ++ ++ private SecureDirectoryStream delegate() { ++ return (SecureDirectoryStream) delegate; ++ } ++} +diff --git a/sshd-common/src/main/java/org/apache/sshd/common/file/util/BaseFileSystem.java b/sshd-common/src/main/java/org/apache/sshd/common/file/util/BaseFileSystem.java +index a5d099e8c..51b9a03bc 100644 +--- a/sshd-common/src/main/java/org/apache/sshd/common/file/util/BaseFileSystem.java ++++ b/sshd-common/src/main/java/org/apache/sshd/common/file/util/BaseFileSystem.java +@@ -34,6 +34,7 @@ + import java.util.regex.Pattern; + + import org.apache.sshd.common.util.GenericUtils; ++import org.apache.sshd.common.util.OsUtils; + import org.slf4j.Logger; + import org.slf4j.LoggerFactory; + +@@ -79,7 +80,7 @@ public Iterable getFileStores() { + public T getPath(String first, String... more) { + StringBuilder sb = new StringBuilder(); + if (!GenericUtils.isEmpty(first)) { +- appendDedupSep(sb, first.replace('\\', '/')); // in case we are running on Windows ++ appendDedupSep(sb, handleWindowsSeparator(first)); + } + + if (GenericUtils.length(more) > 0) { +@@ -87,8 +88,7 @@ public T getPath(String first, String... more) { + if ((sb.length() > 0) && (sb.charAt(sb.length() - 1) != '/')) { + sb.append('/'); + } +- // in case we are running on Windows +- appendDedupSep(sb, segment.replace('\\', '/')); ++ appendDedupSep(sb, handleWindowsSeparator(segment)); + } + } + +@@ -121,6 +121,23 @@ protected void appendDedupSep(StringBuilder sb, CharSequence s) { + } + } + ++ /** ++ * In case we are running on Windows, accept "\\" as a file separator. Ignore in *nix as "\\" is a valid filename ++ * ++ * @param name the name to fix the separator for if running on Windows ++ * @return the fixed name ++ */ ++ protected String handleWindowsSeparator(String name) { ++ if (hostFsHasWindowsSeparator()) { ++ return name.replace('\\', '/'); ++ } ++ return name; ++ } ++ ++ protected boolean hostFsHasWindowsSeparator() { ++ return OsUtils.isWin32(); ++ } ++ + @Override + public PathMatcher getPathMatcher(String syntaxAndPattern) { + int colonIndex = Objects.requireNonNull(syntaxAndPattern, "No argument").indexOf(':'); +diff --git a/sshd-common/src/main/java/org/apache/sshd/common/util/io/IoUtils.java b/sshd-common/src/main/java/org/apache/sshd/common/util/io/IoUtils.java +index ccac66938..11dd1d92d 100644 +--- a/sshd-common/src/main/java/org/apache/sshd/common/util/io/IoUtils.java ++++ b/sshd-common/src/main/java/org/apache/sshd/common/util/io/IoUtils.java +@@ -47,6 +47,7 @@ + import java.util.Collection; + import java.util.Collections; + import java.util.EnumSet; ++import java.util.Iterator; + import java.util.List; + import java.util.Objects; + import java.util.Set; +@@ -394,18 +395,13 @@ public static String getFileOwner(Path path, LinkOption... options) throws IOExc + * explained above + */ + public static Boolean checkFileExists(Path path, LinkOption... options) { +- boolean followLinks = true; +- for (LinkOption opt : options) { +- if (opt == LinkOption.NOFOLLOW_LINKS) { +- followLinks = false; +- break; +- } +- } ++ boolean followLinks = followLinks(options); ++ + try { + if (followLinks) { + path.getFileSystem().provider().checkAccess(path); + } else { +- Files.readAttributes(path, BasicFileAttributes.class, LinkOption.NOFOLLOW_LINKS); ++ Files.readAttributes(path, BasicFileAttributes.class, options); + } + return Boolean.TRUE; + } catch (NoSuchFileException e) { +@@ -415,6 +411,63 @@ public static Boolean checkFileExists(Path path, LinkOption... options) { + } + } + ++ /** ++ * Checks that a file exists with or without following any symlinks. ++ * ++ * @param path the path to check ++ * @param neverFollowSymlinks whether to follow symlinks ++ * @return true if the file exists with the symlink semantics, false if it doesn't exist, null ++ * if symlinks were found, or it is unknown if whether the file exists ++ */ ++ public static Boolean checkFileExistsAnySymlinks(Path path, boolean neverFollowSymlinks) { ++ try { ++ if (!neverFollowSymlinks) { ++ path.getFileSystem().provider().checkAccess(path); ++ } else { ++ // this is a bad fix because this leaves a nasty race condition - the directory may turn into a symlink ++ // between this check and the call to open() ++ for (int i = 1; i <= path.getNameCount(); i++) { ++ Path checkForSymLink = getFirstPartsOfPath(path, i); ++ BasicFileAttributes basicFileAttributes ++ = Files.readAttributes(checkForSymLink, BasicFileAttributes.class, LinkOption.NOFOLLOW_LINKS); ++ if (basicFileAttributes.isSymbolicLink()) { ++ return false; ++ } ++ } ++ } ++ return true; ++ } catch (NoSuchFileException e) { ++ return false; ++ } catch (IOException e) { ++ return null; ++ } ++ } ++ ++ /** ++ * Extracts the first n parts of the path. For example
++ * ("/home/test/test12", 1) returns "/home",
++ * ("/home/test", 1) returns "/home/test"
++ * etc. ++ * ++ * @param path the path to extract parts of ++ * @param partsToExtract the number of parts to extract ++ * @return the extracted path ++ */ ++ public static Path getFirstPartsOfPath(Path path, int partsToExtract) { ++ String firstName = path.getName(0).toString(); ++ String[] names = new String[partsToExtract - 1]; ++ for (int j = 1; j < partsToExtract; j++) { ++ names[j - 1] = path.getName(j).toString(); ++ } ++ Path checkForSymLink = path.getFileSystem().getPath(firstName, names); ++ // the root is not counted as a directory part so we must resolve the result relative to it. ++ Path root = path.getRoot(); ++ if (root != null) { ++ checkForSymLink = root.resolve(checkForSymLink); ++ } ++ return checkForSymLink; ++ } ++ + /** + * Read the requested number of bytes or fail if there are not enough left. + * +@@ -637,4 +690,95 @@ public static List readAllLines(BufferedReader reader, int lineCountHint + } + return result; + } ++ ++ /** ++ * Chroot a path under the new root ++ * ++ * @param newRoot the new root ++ * @param toSanitize the path to sanitize and chroot ++ * @return the chrooted path under the newRoot filesystem ++ */ ++ public static Path chroot(Path newRoot, Path toSanitize) { ++ Objects.requireNonNull(newRoot); ++ Objects.requireNonNull(toSanitize); ++ List sanitized = removeExtraCdUps(toSanitize); ++ return buildPath(newRoot, newRoot.getFileSystem(), sanitized); ++ } ++ ++ /** ++ * Remove any extra directory ups from the Path ++ * ++ * @param toSanitize the path to sanitize ++ * @return the sanitized path ++ */ ++ public static Path removeCdUpAboveRoot(Path toSanitize) { ++ List sanitized = removeExtraCdUps(toSanitize); ++ return buildPath(toSanitize.getRoot(), toSanitize.getFileSystem(), sanitized); ++ } ++ ++ private static List removeExtraCdUps(Path toResolve) { ++ List newNames = new ArrayList<>(toResolve.getNameCount()); ++ ++ int numCdUps = 0; ++ int numDirParts = 0; ++ for (int i = 0; i < toResolve.getNameCount(); i++) { ++ String name = toResolve.getName(i).toString(); ++ if ("..".equals(name)) { ++ // If we have more cdups than dir parts, so we ignore the ".." to avoid jail escapes ++ if (numDirParts > numCdUps) { ++ ++numCdUps; ++ newNames.add(name); ++ } ++ } else { ++ // if the current directory is a part of the name, don't increment number of dir parts, as it doesn't ++ // add to the number of ".."s that can be present before the root ++ if (!".".equals(name)) { ++ ++numDirParts; ++ } ++ newNames.add(name); ++ } ++ } ++ return newNames; ++ } ++ ++ /** ++ * Build a path from the list of path parts ++ * ++ * @param root the root path ++ * @param fs the filesystem ++ * @param namesList the parts of the path to build ++ * @return the built path ++ */ ++ public static Path buildPath(Path root, FileSystem fs, List namesList) { ++ Objects.requireNonNull(fs); ++ if (namesList == null) { ++ return null; ++ } ++ ++ if (GenericUtils.isEmpty(namesList)) { ++ return root == null ? fs.getPath(".") : root; ++ } ++ ++ Path cleanedPathToResolve = buildRelativePath(fs, namesList); ++ return root == null ? cleanedPathToResolve : root.resolve(cleanedPathToResolve); ++ } ++ ++ /** ++ * Build a relative path on the filesystem fs from the path parts in the namesList ++ * ++ * @param fs the filesystem for the path ++ * @param namesList the names list ++ * @return the built path ++ */ ++ public static Path buildRelativePath(FileSystem fs, List namesList) { ++ String[] names = new String[namesList.size() - 1]; ++ ++ Iterator it = namesList.iterator(); ++ String rootName = it.next(); ++ for (int i = 0; it.hasNext(); i++) { ++ names[i] = it.next(); ++ } ++ Path cleanedPathToResolve = fs.getPath(rootName, names); ++ return cleanedPathToResolve; ++ } + } +diff --git a/sshd-common/src/test/java/org/apache/sshd/common/file/root/RootedFileSystemProviderTest.java b/sshd-common/src/test/java/org/apache/sshd/common/file/root/RootedFileSystemProviderTest.java +index 9ec46b830..2520753ab 100644 +--- a/sshd-common/src/test/java/org/apache/sshd/common/file/root/RootedFileSystemProviderTest.java ++++ b/sshd-common/src/test/java/org/apache/sshd/common/file/root/RootedFileSystemProviderTest.java +@@ -19,28 +19,29 @@ + + package org.apache.sshd.common.file.root; + ++import java.io.File; + import java.io.IOException; ++import java.io.InputStream; ++import java.io.OutputStream; + import java.nio.ByteBuffer; + import java.nio.channels.Channel; + import java.nio.channels.FileChannel; +-import java.nio.file.DirectoryStream; +-import java.nio.file.FileSystem; +-import java.nio.file.Files; +-import java.nio.file.InvalidPathException; +-import java.nio.file.OpenOption; +-import java.nio.file.Path; +-import java.nio.file.StandardCopyOption; +-import java.nio.file.StandardOpenOption; ++import java.nio.charset.StandardCharsets; ++import java.nio.file.*; ++import java.util.ArrayList; + import java.util.Arrays; + import java.util.Collections; ++import java.util.List; + import java.util.Objects; + import java.util.Random; + import java.util.TreeSet; + + import org.apache.sshd.common.util.GenericUtils; ++import org.apache.sshd.common.util.OsUtils; + import org.apache.sshd.util.test.CommonTestSupportUtils; + import org.apache.sshd.util.test.NoIoTestCase; +-import org.junit.BeforeClass; ++import org.junit.Assert; ++import org.junit.Assume; + import org.junit.FixMethodOrder; + import org.junit.Test; + import org.junit.experimental.categories.Category; +@@ -49,7 +50,7 @@ + /** + * Tests the RootedFileSystemProvider implementation of {@link java.nio.file.spi.FileSystemProvider} checking that + * permissions for generic FS commands are not permitted outside of the root directory. +- * ++ *

+ * Individual tests are form pairs (e.g. testX, testXInvalid) where testXInvalid is expected to test a parent path of + * {@link RootedFileSystem#getRoot()} + * +@@ -58,19 +59,21 @@ + @FixMethodOrder(MethodSorters.NAME_ASCENDING) + @Category({ NoIoTestCase.class }) + public class RootedFileSystemProviderTest extends AssertableFile { +- private static RootedFileSystem fileSystem; +- private static Path rootSandbox; ++ private static final String SKIP_ON_WINDOWS = "Test fails due to windows normalizing paths before opening them, " + ++ "allowing one to open a file like \"C:\\directory_doesnt_exist\\..\\myfile.txt\" whereas this is blocked in unix"; ++ private static final String DOESNT_EXIST = "../doesnt_exist/../"; + +- public RootedFileSystemProviderTest() { +- super(); +- } ++ private final RootedFileSystem fileSystem; ++ private final Path rootSandbox; ++ private final FileHelper fileHelper; + +- @BeforeClass +- public static void initializeFileSystem() throws IOException { ++ public RootedFileSystemProviderTest() throws Exception { ++ super(); ++ fileHelper = new FileHelper(); + Path targetFolder = Objects.requireNonNull( + CommonTestSupportUtils.detectTargetFolder(RootedFileSystemProviderTest.class), + "Failed to detect target folder"); +- rootSandbox = FileHelper.createTestSandbox(targetFolder.resolve(TEMP_SUBFOLDER_NAME)); ++ rootSandbox = fileHelper.createTestSandbox(targetFolder.resolve(TEMP_SUBFOLDER_NAME)); + fileSystem = (RootedFileSystem) new RootedFileSystemProvider().newFileSystem(rootSandbox, Collections.emptyMap()); + } + +@@ -86,144 +89,216 @@ public void testRoot() { + /* mkdir */ + @Test + public void testMkdir() throws IOException { +- Path created = FileHelper.createDirectory(fileSystem.getPath(getCurrentTestName())); +- assertTrue(exists(created) && isDir(created) && isReadable(created)); ++ Path created = fileHelper.createDirectory(fileSystem.getPath(getCurrentTestName())); ++ try { ++ assertTrue(exists(created) && isDir(created) && isReadable(created)); ++ } finally { ++ Files.delete(created); ++ } + } + +- @Test(expected = InvalidPathException.class) +- public void testMkdirInvalid() throws IOException { +- Path parent = FileHelper.createDirectory(fileSystem.getPath("../" + getCurrentTestName())); +- fail(String.format("Unexpected success in creating directory %s", parent.toString())); ++ @Test ++ public void testMkdirInvalid() { ++ Assume.assumeFalse(SKIP_ON_WINDOWS, OsUtils.isWin32()); ++ ++ String parent = DOESNT_EXIST + getCurrentTestName(); ++ assertThrows(String.format("Unexpected success in creating directory %s", parent), NoSuchFileException.class, ++ () -> fileHelper.createDirectory(fileSystem.getPath(parent))); + } + + /* rmdir */ + @Test + public void testRmdir() throws IOException { +- Path created = FileHelper.createDirectory(fileSystem.getPath(getCurrentTestName())); +- Path deleted = FileHelper.deleteDirectory(created); ++ Path created = fileHelper.createDirectory(fileSystem.getPath(getCurrentTestName())); ++ Path deleted = fileHelper.deleteDirectory(created); + notExists(deleted); + } + +- @Test(expected = InvalidPathException.class) ++ @Test(expected = NoSuchFileException.class) + public void testRmdirInvalid() throws IOException { +- Path deleted = FileHelper.deleteDirectory(fileSystem.getPath("../" + getCurrentTestName())); ++ Path deleted = fileHelper.deleteDirectory(fileSystem.getPath(DOESNT_EXIST + getCurrentTestName())); + fail(String.format("Unexpected success in removing directory %s", deleted.toString())); + } + + /* chdir */ + @Test + public void testChdir() throws IOException { +- Path created = FileHelper.createDirectory(fileSystem.getPath(getCurrentTestName())); +- Path createdFile = FileHelper.createFile(created.resolve(getCurrentTestName())); +- boolean hasFile = false; +- try (DirectoryStream ds = FileHelper.readDirectory(created)) { +- for (Path p : ds) { +- hasFile |= FileHelper.isSameFile(createdFile, +- fileSystem.getPath(created.getFileName() + "/" + p.getFileName())); ++ Path created = fileHelper.createDirectory(fileSystem.getPath(getCurrentTestName())); ++ Path createdFile = fileHelper.createFile(created.resolve(getCurrentTestName())); ++ try { ++ boolean hasFile = false; ++ try (DirectoryStream ds = fileHelper.readDirectory(created)) { ++ for (Path p : ds) { ++ hasFile |= fileHelper.isSameFile(createdFile, ++ fileSystem.getPath(created.getFileName() + "/" + p.getFileName())); ++ } + } ++ assertTrue(createdFile + " found in ch directory", hasFile); ++ } finally { ++ Files.delete(createdFile); ++ Files.delete(created); + } +- assertTrue(createdFile + " found in ch directory", hasFile); +- } +- +- @Test(expected = InvalidPathException.class) +- public void testChdirInvalid() throws IOException { +- Path chdir = FileHelper.createDirectory(fileSystem.getPath("../" + getCurrentTestName())); +- fail(String.format("Unexpected success in changing directory %s", chdir.toString())); + } + + /* write */ + @Test + public void testWriteFile() throws IOException { +- Path created = FileHelper.createFile(fileSystem.getPath(getCurrentTestName())); ++ Path created = fileHelper.createFile(fileSystem.getPath(getCurrentTestName())); + assertTrue(exists(created) && isReadable(created)); + } + +- @Test(expected = InvalidPathException.class) ++ @Test + public void testWriteFileInvalid() throws IOException { +- Path written = FileHelper.createFile(fileSystem.getPath("../" + getCurrentTestName())); +- fail(String.format("Unexpected success in writing file %s", written.toString())); ++ Assume.assumeFalse(SKIP_ON_WINDOWS, OsUtils.isWin32()); ++ ++ String written = DOESNT_EXIST + getCurrentTestName(); ++ assertThrows(String.format("Unexpected success in writing file %s", written), NoSuchFileException.class, ++ () -> fileHelper.createFile(fileSystem.getPath(written))); + } + + /* read */ + @Test + public void testReadFile() throws IOException { +- Path created = FileHelper.createFile(fileSystem.getPath(getCurrentTestName())); +- isNonEmpty(FileHelper.readFile(created)); ++ Path created = fileHelper.createFile(fileSystem.getPath(getCurrentTestName())); ++ isNonEmpty(fileHelper.readFile(created)); + } + +- @Test(expected = InvalidPathException.class) ++ @Test(expected = NoSuchFileException.class) + public void testReadFileInvalid() throws IOException { +- Path read = fileSystem.getPath("../" + getCurrentTestName()); +- FileHelper.readFile(read); ++ Path read = fileSystem.getPath(DOESNT_EXIST + getCurrentTestName()); ++ fileHelper.readFile(read); + fail(String.format("Unexpected success in reading file %s", read.toString())); + } + + /* rm */ + @Test + public void testDeleteFile() throws IOException { +- Path created = FileHelper.createFile(fileSystem.getPath(getCurrentTestName())); +- Path deleted = FileHelper.deleteFile(created); ++ Path created = fileHelper.createFile(fileSystem.getPath(getCurrentTestName())); ++ Path deleted = fileHelper.deleteFile(created); + notExists(deleted); + } + +- @Test(expected = InvalidPathException.class) ++ @Test(expected = NoSuchFileException.class) + public void testDeleteFileInvalid() throws IOException { +- Path deleted = FileHelper.deleteFile(fileSystem.getPath("../" + getCurrentTestName())); ++ Path deleted = fileHelper.deleteFile(fileSystem.getPath(DOESNT_EXIST + getCurrentTestName())); + fail(String.format("Unexpected success in deleting file %s", deleted.toString())); + } + + /* cp */ + @Test + public void testCopyFile() throws IOException { +- Path created = FileHelper.createFile(fileSystem.getPath(getCurrentTestName())); ++ Path created = fileHelper.createFile(fileSystem.getPath(getCurrentTestName())); + Path destination = fileSystem.getPath(getCurrentTestName() + "dest"); +- FileHelper.copyFile(created, destination); +- assertTrue(exists(destination) && isReadable(destination)); ++ try { ++ fileHelper.copyFile(created, destination); ++ assertTrue(exists(destination) && isReadable(destination)); ++ } finally { ++ Files.delete(destination); ++ Files.delete(created); ++ } + } + +- @Test(expected = InvalidPathException.class) ++ @Test + public void testCopyFileInvalid() throws IOException { +- Path created = FileHelper.createFile(fileSystem.getPath(getCurrentTestName())); +- Path copy = FileHelper.copyFile(created, fileSystem.getPath("../" + getCurrentTestName())); +- fail(String.format("Unexpected success in copying file to %s", copy.toString())); ++ Assume.assumeFalse(SKIP_ON_WINDOWS, OsUtils.isWin32()); ++ ++ Path created = fileHelper.createFile(fileSystem.getPath(getCurrentTestName())); ++ String copy = DOESNT_EXIST + getCurrentTestName(); ++ assertThrows(String.format("Unexpected success in copying file to %s", copy), ++ NoSuchFileException.class, ++ () -> fileHelper.copyFile(created, fileSystem.getPath(copy))); + } + + /* mv */ + @Test + public void testMoveFile() throws IOException { +- Path created = FileHelper.createFile(fileSystem.getPath(getCurrentTestName())); ++ Path created = fileHelper.createFile(fileSystem.getPath(getCurrentTestName())); + Path destination = fileSystem.getPath(getCurrentTestName() + "dest"); +- FileHelper.moveFile(created, destination); ++ fileHelper.moveFile(created, destination); + assertTrue(notExists(created) && exists(destination) && isReadable(destination)); + } + +- @Test(expected = InvalidPathException.class) ++ @Test + public void testMoveFileInvalid() throws IOException { +- Path created = FileHelper.createFile(fileSystem.getPath(getCurrentTestName())); +- Path moved = FileHelper.moveFile(created, fileSystem.getPath("../" + getCurrentTestName())); +- fail(String.format("Unexpected success in moving file to %s", moved.toString())); ++ Assume.assumeFalse(SKIP_ON_WINDOWS, OsUtils.isWin32()); ++ ++ Path created = fileHelper.createFile(fileSystem.getPath(getCurrentTestName())); ++ String moved = DOESNT_EXIST + getCurrentTestName(); ++ assertThrows(String.format("Unexpected success in moving file to %s", moved), NoSuchFileException.class, ++ () -> fileHelper.moveFile(created, fileSystem.getPath(moved))); + } + + /* link */ + @Test + public void testCreateLink() throws IOException { +- Path existing = FileHelper.createFile(fileSystem.getPath(getCurrentTestName())); ++ Path existing = fileHelper.createFile(fileSystem.getPath(getCurrentTestName())); + Path link = fileSystem.getPath(getCurrentTestName() + "link"); +- FileHelper.createLink(link, existing); +- assertTrue(exists(link) && isReadable(link)); ++ try { ++ fileHelper.createLink(link, existing); ++ assertTrue(exists(link) && isReadable(link)); ++ } finally { ++ Files.delete(link); ++ Files.delete(existing); ++ } ++ } ++ ++ @Test ++ public void testJailbreakLink() throws IOException { ++ testJailbreakLink("../"); ++ } ++ ++ @Test ++ public void testJailbreakLink2() throws IOException { ++ testJailbreakLink("../test/"); ++ } ++ ++ @Test ++ public void testJailbreakLink3() throws IOException { ++ testJailbreakLink("/.."); ++ } ++ ++ @Test ++ public void testJailbreakLink4() throws IOException { ++ testJailbreakLink("/./.."); ++ } ++ ++ @Test ++ public void testJailbreakLink5() throws IOException { ++ testJailbreakLink("/./../"); ++ } ++ ++ @Test ++ public void testJailbreakLink6() throws IOException { ++ testJailbreakLink("./../"); ++ } ++ ++ @Test ++ public void testJailbreakLink7() throws IOException { ++ String fileName = "/testdir/testdir2/../../.."; ++ testJailbreakLink(fileName); + } + +- @Test(expected = InvalidPathException.class) ++ private void testJailbreakLink(String jailbrokenTarget) throws IOException { ++ Path target = fileSystem.getPath(jailbrokenTarget); ++ Path linkPath = fileSystem.getPath("/testLink"); ++ Assert.assertThrows(InvalidPathException.class, () -> fileSystem.provider().createSymbolicLink(linkPath, target)); ++ Assert.assertFalse(Files.exists(linkPath)); ++ } ++ ++ @Test + public void testCreateLinkInvalid() throws IOException { +- Path existing = FileHelper.createFile(fileSystem.getPath(getCurrentTestName())); +- Path link = FileHelper.createLink(fileSystem.getPath("../" + getCurrentTestName() + "link"), existing); +- fail(String.format("Unexpected success in linking file %s", link.toString())); ++ Assume.assumeFalse(SKIP_ON_WINDOWS, OsUtils.isWin32()); ++ ++ Path existing = fileHelper.createFile(fileSystem.getPath(getCurrentTestName())); ++ String link = DOESNT_EXIST + getCurrentTestName() + "link"; ++ assertThrows(String.format("Unexpected success in linking file %s", link), NoSuchFileException.class, ++ () -> fileHelper.createLink(fileSystem.getPath(link), existing)); + } + + @Test + public void testNewByteChannelProviderMismatchException() throws IOException { + RootedFileSystemProvider provider = new RootedFileSystemProvider(); +- Path tempFolder = assertHierarchyTargetFolderExists(getTempTargetFolder()); ++ Path tempFolder = getTempTargetFolder(); + Path file = Files.createTempFile(tempFolder, getCurrentTestName(), ".txt"); + try (FileSystem fs = provider.newFileSystem(tempFolder, Collections.emptyMap()); + Channel channel = provider.newByteChannel(fs.getPath(file.getFileName().toString()), Collections.emptySet())) { +@@ -235,23 +310,212 @@ public void testNewByteChannelProviderMismatchException() throws IOException { + public void testResolveRoot() throws IOException { + Path root = GenericUtils.head(fileSystem.getRootDirectories()); + Path dir = root.resolve("tsd"); +- FileHelper.createDirectory(dir); +- Path f1 = FileHelper.createFile(dir.resolve("test.txt")); +- Path f2 = Files.newDirectoryStream(dir).iterator().next(); +- assertTrue("Unrooted path found", f2 instanceof RootedPath); +- assertEquals(f1, f2); +- FileHelper.deleteFile(f1); +- FileHelper.deleteDirectory(dir); ++ fileHelper.createDirectory(dir); ++ Path f1 = fileHelper.createFile(dir.resolve("test.txt")); ++ try { ++ Path f2; ++ try (DirectoryStream ds = Files.newDirectoryStream(dir)) { ++ f2 = ds.iterator().next(); ++ } ++ assertTrue("Unrooted path found", f2 instanceof RootedPath); ++ assertEquals(f1, f2); ++ } finally { ++ fileHelper.deleteFile(f1); ++ fileHelper.deleteDirectory(dir); ++ } ++ } ++ ++ @Test ++ public void testBreakOutOfChroot1() throws IOException { ++ String fileName = "../" + getCurrentTestName(); ++ testBreakOutOfChroot(fileName, fileName); ++ } ++ ++ @Test ++ public void testBreakOutOfChroot2() throws IOException { ++ String fileName = "./../" + getCurrentTestName(); ++ testBreakOutOfChroot(fileName, fileName); ++ } ++ ++ @Test ++ public void testBreakOutOfChroot3() throws IOException { ++ String fileName = "/../" + getCurrentTestName(); ++ testBreakOutOfChroot(fileName, fileName); ++ } ++ ++ @Test ++ public void testBreakOutOfChroot4() throws IOException { ++ String fileName = "/.././" + getCurrentTestName(); ++ testBreakOutOfChroot(fileName, fileName); ++ } ++ ++ @Test ++ public void testBreakOutOfChroot5() throws IOException { ++ String fileName = "/./../" + getCurrentTestName(); ++ testBreakOutOfChroot(fileName, fileName); ++ } ++ ++ @Test ++ public void testBreakOutOfChroot6() throws IOException { ++ String fileName = "//../" + getCurrentTestName(); ++ testBreakOutOfChroot(fileName, "/../" + getCurrentTestName()); ++ } ++ ++ /** ++ * Tests to make sure that the attempted break out of the chroot does not work with the specified filename ++ * ++ * @param fileName the filename to attempt to break out of the chroot with ++ * @throws IOException on test failure ++ */ ++ private void testBreakOutOfChroot(String fileName, String expected) throws IOException { ++ RootedPath breakoutAttempt = fileSystem.getPath(fileName); ++ ++ // make sure that our rooted fs behaves like a proper unix fs ++ Assert.assertEquals(expected, breakoutAttempt.toString()); ++ ++ Path expectedDir = fileSystem.getRoot().resolve(getCurrentTestName()); ++ Path newDir = fileHelper.createDirectory(breakoutAttempt); ++ try { ++ assertTrue(Files.isDirectory(expectedDir)); ++ ++ String baseName = breakoutAttempt.getName(breakoutAttempt.getNameCount() - 1).toString(); ++ assertTrue(fileHelper.isSameFile(newDir, fileSystem.getPath(baseName))); ++ ++ // make sure we didn't create it one directory out of the jail ++ assertFalse(Files.exists(fileSystem.getRoot().resolve("../" + breakoutAttempt.getFileName().toString()))); ++ ++ // make sure various methods of referencing the file work. ++ assertTrue(fileHelper.isSameFile(newDir, fileSystem.getPath("/" + baseName))); ++ assertTrue(fileHelper.isSameFile(newDir, fileSystem.getPath("/../../" + baseName))); ++ assertTrue(fileHelper.isSameFile(newDir, fileSystem.getPath("./../" + baseName))); ++ } finally { ++ // cleanup the directory. ++ fileHelper.deleteDirectory(newDir); ++ } ++ ++ assertFalse(Files.isDirectory(expectedDir)); ++ assertFalse(Files.isDirectory(newDir)); ++ } ++ ++ @Test ++ public void testValidSymlink1() throws IOException { ++ Assume.assumeFalse(SKIP_ON_WINDOWS, OsUtils.isWin32()); ++ String fileName = "/testdir/../"; ++ testValidSymlink(fileName, true); ++ } ++ ++ @Test ++ public void testValidSymlink2() throws IOException { ++ Assume.assumeFalse(SKIP_ON_WINDOWS, OsUtils.isWin32()); ++ String fileName = "/testdir/testdir2/../"; ++ testValidSymlink(fileName, true); ++ } ++ ++ @Test ++ public void testValidSymlink3() throws IOException { ++ Assume.assumeFalse(SKIP_ON_WINDOWS, OsUtils.isWin32()); ++ String fileName = "/testdir/../testdir3/"; ++ testValidSymlink(fileName, true); ++ } ++ ++ @Test ++ public void testValidSymlink4() throws IOException { ++ Assume.assumeFalse(SKIP_ON_WINDOWS, OsUtils.isWin32()); ++ String fileName = "testdir/../testdir3/../"; ++ testValidSymlink(fileName, true); ++ } ++ ++ @Test ++ public void testValidSymlink5() throws IOException { ++ Assume.assumeFalse(SKIP_ON_WINDOWS, OsUtils.isWin32()); ++ String fileName = "testdir/../testdir3/../testfile"; ++ testValidSymlink(fileName, false); ++ } ++ ++ public void testValidSymlink(String symlink, boolean targetIsDirectory) throws IOException { ++ Path target = fileSystem.getPath(symlink); ++ Path linkPath = fileSystem.getPath("/testLink"); ++ final List toDelete = new ArrayList<>(); ++ try { ++ fileSystem.provider().createSymbolicLink(linkPath, target); ++ toDelete.add(linkPath); ++ ++ // ensure that nothing processed the symlink. ++ Assert.assertEquals(Paths.get(symlink).toString(), ++ fileSystem.provider().readSymbolicLink(linkPath).toString()); ++ Assert.assertFalse(Files.exists(target)); ++ Assert.assertEquals(Files.exists(linkPath), Files.exists(target)); ++ ++ // If we don't follow the link, we simply check that the link exists, which it does as we created it. ++ Assert.assertTrue(Files.exists(linkPath, LinkOption.NOFOLLOW_LINKS)); ++ ++ createParentDirs(targetIsDirectory ? target : target.getParent(), toDelete); ++ ++ if (!targetIsDirectory) { ++ Files.createFile(target); ++ toDelete.add(target); ++ } ++ ++ Assert.assertTrue(Files.exists(linkPath)); ++ } finally { ++ for (int i = toDelete.size() - 1; i >= 0; i--) { ++ Path path = toDelete.get(i); ++ try { ++ Files.delete(path); ++ } catch (IOException ex) { ++ // ignore as we might try to delete "/dir/.." which will fail as it contains dir.. ++ } ++ } ++ } ++ } ++ ++ private static void createParentDirs(Path target, List toDelete) throws IOException { ++ if (target.getParent() != null) { ++ createParentDirs(target.getParent(), toDelete); ++ } ++ ++ if (!Files.isDirectory(target)) { ++ Files.createDirectories(target); ++ toDelete.add(target); ++ } ++ } ++ ++ @Test ++ public void testFileNamedSlashOnUnixBasedOS() throws IOException { ++ // skip ths test on Win32 ++ if (!"\\".equals(File.separator)) { ++ Path slashFile = fileSystem.getPath("\\"); ++ Path created = fileHelper.createFile(slashFile); ++ try { ++ assertTrue(Files.isRegularFile(created)); ++ } finally { ++ fileHelper.deleteFile(created); ++ } ++ } ++ } ++ ++ @Test ++ public void testStreams() throws IOException { ++ byte[] data = "This is test data".getBytes(StandardCharsets.UTF_8); ++ RootedPath testPath = fileSystem.getPath("testfile.txt"); ++ try (OutputStream is = Files.newOutputStream(testPath)) { ++ is.write(data); ++ } ++ byte[] read = new byte[data.length]; ++ try (InputStream is = Files.newInputStream(testPath)) { ++ is.read(read); ++ } ++ assertArrayEquals(data, read); + } + + /* Private helper */ + + /** + * Wrapper around the FileSystemProvider to test generic FS related commands. All created temp directories and files +- * used for testing are deleted upon JVM exit. ++ * used for testing must be deleted in the test which creates them + */ + @SuppressWarnings("synthetic-access") +- private static final class FileHelper { ++ private final class FileHelper { + private FileHelper() { + super(); + } +@@ -263,71 +527,66 @@ private FileHelper() { + * @return the created sandbox Path + * @throws IOException on failure to create + */ +- public static Path createTestSandbox(Path tempDir) throws IOException { +- Path created = Files.createDirectories(tempDir.resolve(RootedFileSystemProviderTest.class.getSimpleName())); +- created.toFile().deleteOnExit(); ++ public Path createTestSandbox(Path tempDir) throws IOException { ++ Path path = tempDir.resolve(RootedFileSystemProviderTest.class.getSimpleName()); ++ Path created = Files.createDirectories(path); + return created; + } + +- public static Path createFile(Path source) throws InvalidPathException, IOException { ++ public Path createFile(Path source) throws InvalidPathException, IOException { + try (FileChannel fc = fileSystem.provider().newFileChannel(source, + new TreeSet(Arrays.asList(StandardOpenOption.CREATE, StandardOpenOption.WRITE)))) { + byte[] randomBytes = new byte[1000]; + new Random().nextBytes(randomBytes); + fc.write(ByteBuffer.wrap(randomBytes)); +- source.toFile().deleteOnExit(); + return source; + } + } + +- public static Path createLink(Path link, Path existing) throws IOException { ++ public Path createLink(Path link, Path existing) throws IOException { + fileSystem.provider().createLink(link, existing); +- link.toFile().deleteOnExit(); + return link; + } + +- public static Path createDirectory(Path dir) throws InvalidPathException, IOException { ++ public Path createDirectory(Path dir) throws InvalidPathException, IOException { + fileSystem.provider().createDirectory(dir); +- dir.toFile().deleteOnExit(); + return dir; + } + +- public static Path deleteDirectory(Path dir) throws InvalidPathException, IOException { ++ public Path deleteDirectory(Path dir) throws InvalidPathException, IOException { + return deleteFile(dir); + } + +- public static Path deleteFile(Path source) throws InvalidPathException, IOException { ++ public Path deleteFile(Path source) throws InvalidPathException, IOException { + fileSystem.provider().delete(source); + return source; + } + +- public static byte[] readFile(Path source) throws IOException { ++ public byte[] readFile(Path source) throws IOException { + try (FileChannel fc = fileSystem.provider().newFileChannel(source, +- new TreeSet(Arrays.asList(StandardOpenOption.READ)))) { +- byte[] readBytes = new byte[(int) source.toFile().length()]; ++ Collections.singleton(StandardOpenOption.READ))) { ++ byte[] readBytes = new byte[(int) Files.size(source)]; + fc.read(ByteBuffer.wrap(readBytes)); + return readBytes; + } + } + +- public static Path copyFile(Path source, Path destination) throws InvalidPathException, IOException { ++ public Path copyFile(Path source, Path destination) throws InvalidPathException, IOException { + fileSystem.provider().copy(source, destination, StandardCopyOption.COPY_ATTRIBUTES); +- destination.toFile().deleteOnExit(); + return destination; + } + +- public static Path moveFile(Path source, Path destination) throws InvalidPathException, IOException { ++ public Path moveFile(Path source, Path destination) throws InvalidPathException, IOException { + fileSystem.provider().move(source, destination, StandardCopyOption.ATOMIC_MOVE); +- destination.toFile().deleteOnExit(); + return destination; + } + +- public static DirectoryStream readDirectory(Path dir) throws InvalidPathException, IOException { ++ public DirectoryStream readDirectory(Path dir) throws InvalidPathException, IOException { + DirectoryStream dirStream = fileSystem.provider().newDirectoryStream(dir, entry -> true); + return dirStream; + } + +- public static boolean isSameFile(Path source, Path destination) throws IOException { ++ public boolean isSameFile(Path source, Path destination) throws IOException { + return fileSystem.provider().isSameFile(source, destination); + } + } +diff --git a/sshd-common/src/test/java/org/apache/sshd/common/util/io/IoUtilsTest.java b/sshd-common/src/test/java/org/apache/sshd/common/util/io/IoUtilsTest.java +index a94811dc5..247e3d655 100644 +--- a/sshd-common/src/test/java/org/apache/sshd/common/util/io/IoUtilsTest.java ++++ b/sshd-common/src/test/java/org/apache/sshd/common/util/io/IoUtilsTest.java +@@ -19,11 +19,17 @@ + + package org.apache.sshd.common.util.io; + ++import java.io.IOException; ++import java.nio.file.Files; + import java.nio.file.LinkOption; ++import java.nio.file.Path; ++import java.nio.file.Paths; + + import org.apache.sshd.common.util.NumberUtils; ++import org.apache.sshd.util.test.CommonTestSupportUtils; + import org.apache.sshd.util.test.JUnitTestSupport; + import org.apache.sshd.util.test.NoIoTestCase; ++import org.junit.Assert; + import org.junit.FixMethodOrder; + import org.junit.Test; + import org.junit.experimental.categories.Category; +@@ -58,4 +64,46 @@ public void testGetEOLBytes() { + } + } + ++ /** ++ * Tests to make sure check exists does not follow symlinks. ++ * ++ * @throws IOException on failure ++ */ ++ @Test ++ public void testCheckExists() throws IOException { ++ testCheckExists(Paths.get("target/IoUtilsTest").toAbsolutePath()); ++ } ++ ++ public void testCheckExists(Path baseDir) throws IOException { ++ CommonTestSupportUtils.deleteRecursive(baseDir, LinkOption.NOFOLLOW_LINKS); ++ ++ Path folder = baseDir.resolve("folder1/folder2/"); ++ Files.createDirectories(folder); ++ ++ Path target = baseDir.resolve("folder1/target"); ++ Files.createDirectories(target); ++ ++ Path dirInTarget = baseDir.resolve("folder1/target/dirintarget"); ++ Files.createDirectories(dirInTarget); ++ ++ Files.createDirectories(target); ++ Path link = baseDir.resolve("folder1/folder2/link"); ++ Files.createSymbolicLink(link, target); ++ ++ Path link2 = baseDir.resolve("link"); ++ Files.createSymbolicLink(link2, target); ++ ++ Path targetWithLink = baseDir.resolve("folder1/folder2/link/dirintarget"); ++ ++ Assert.assertTrue("symlink follow should work", IoUtils.checkFileExists(targetWithLink)); ++ Assert.assertTrue("symlink follow should work", IoUtils.checkFileExistsAnySymlinks(targetWithLink, false)); ++ ++ Assert.assertFalse("Link at end shouldn't be followed", IoUtils.checkFileExistsAnySymlinks(link, true)); ++ Assert.assertFalse("Nofollow shouldn't follow directory", ++ IoUtils.checkFileExistsAnySymlinks(targetWithLink, true)); ++ Assert.assertFalse("Link at beginning shouldn't be followed", ++ IoUtils.checkFileExistsAnySymlinks(link2, true)); ++ Assert.assertTrue("Root directory must exist", ++ IoUtils.checkFileExistsAnySymlinks(baseDir, true)); ++ } + } +diff --git a/sshd-common/src/test/java/org/apache/sshd/util/test/CommonTestSupportUtils.java b/sshd-common/src/test/java/org/apache/sshd/util/test/CommonTestSupportUtils.java +index 829f25bcd..57f43fbe3 100644 +--- a/sshd-common/src/test/java/org/apache/sshd/util/test/CommonTestSupportUtils.java ++++ b/sshd-common/src/test/java/org/apache/sshd/util/test/CommonTestSupportUtils.java +@@ -495,11 +495,11 @@ private static Path getFile(String resource) { + * @throws IOException If failed to access/remove some file(s) + */ + public static Path deleteRecursive(Path path, LinkOption... options) throws IOException { +- if ((path == null) || (!Files.exists(path))) { ++ if ((path == null) || (!Files.exists(path, options))) { + return path; + } + +- if (Files.isDirectory(path)) { ++ if (Files.isDirectory(path, options)) { + try (DirectoryStream ds = Files.newDirectoryStream(path)) { + for (Path child : ds) { + deleteRecursive(child, options); +diff --git a/sshd-sftp/src/main/java/org/apache/sshd/sftp/client/fs/SftpFileSystemProvider.java b/sshd-sftp/src/main/java/org/apache/sshd/sftp/client/fs/SftpFileSystemProvider.java +index 4ea058e69..23664be71 100644 +--- a/sshd-sftp/src/main/java/org/apache/sshd/sftp/client/fs/SftpFileSystemProvider.java ++++ b/sshd-sftp/src/main/java/org/apache/sshd/sftp/client/fs/SftpFileSystemProvider.java +@@ -621,7 +621,7 @@ public void copy(Path source, Path target, CopyOption... options) throws IOExcep + } + + // delete target if it exists and REPLACE_EXISTING is specified +- Boolean status = IoUtils.checkFileExists(target, linkOptions); ++ Boolean status = IoUtils.checkFileExistsAnySymlinks(target, noFollowLinks); + if (status == null) { + throw new AccessDeniedException("Existence cannot be determined for copy target: " + target); + } +@@ -698,7 +698,7 @@ public void move(Path source, Path target, CopyOption... options) throws IOExcep + } + + // delete target if it exists and REPLACE_EXISTING is specified +- Boolean status = IoUtils.checkFileExists(target, linkOptions); ++ Boolean status = IoUtils.checkFileExistsAnySymlinks(target, noFollowLinks); + if (status == null) { + throw new AccessDeniedException("Existence cannot be determined for move target " + target); + } +diff --git a/sshd-sftp/src/main/java/org/apache/sshd/sftp/server/AbstractSftpSubsystemHelper.java b/sshd-sftp/src/main/java/org/apache/sshd/sftp/server/AbstractSftpSubsystemHelper.java +index 425234e96..035243daf 100644 +--- a/sshd-sftp/src/main/java/org/apache/sshd/sftp/server/AbstractSftpSubsystemHelper.java ++++ b/sshd-sftp/src/main/java/org/apache/sshd/sftp/server/AbstractSftpSubsystemHelper.java +@@ -36,6 +36,7 @@ + import java.nio.file.LinkOption; + import java.nio.file.NoSuchFileException; + import java.nio.file.NotDirectoryException; ++import java.nio.file.OpenOption; + import java.nio.file.Path; + import java.nio.file.StandardCopyOption; + import java.nio.file.StandardOpenOption; +@@ -51,7 +52,6 @@ + import java.util.Collection; + import java.util.Collections; + import java.util.Comparator; +-import java.util.EnumSet; + import java.util.HashSet; + import java.util.LinkedHashMap; + import java.util.LinkedList; +@@ -645,9 +645,11 @@ protected Map doLStat(int id, String path, int flags) throws IOE + * SSH_FXP_LSTAT does not. + */ + SftpFileSystemAccessor accessor = getFileSystemAccessor(); ++ ++ boolean followLinks = resolvePathResolutionFollowLinks(SftpConstants.SSH_FXP_LSTAT, "", p); + LinkOption[] options = accessor.resolveFileAccessLinkOptions( + this, p, SftpConstants.SSH_FXP_LSTAT, "", false); +- return resolveFileAttributes(p, flags, options); ++ return resolveFileAttributes(p, flags, !followLinks, options); + } + + protected void doSetStat( +@@ -675,7 +677,7 @@ protected void doSetStat( + + Path p = resolveFile(path); + if (followLinks == null) { +- followLinks = resolvePathResolutionFollowLinks(cmd, extension, p); ++ followLinks = resolvePathResolutionFollowLinks(SftpConstants.SSH_FXP_SETSTAT, extension, p); + } + doSetAttributes(cmd, extension, p, attrs, followLinks); + } +@@ -1408,12 +1410,14 @@ protected Map doStat(int id, String path, int flags) throws IOEx + */ + Path p = resolveFile(path); + SftpFileSystemAccessor accessor = getFileSystemAccessor(); ++ boolean followLinks = resolvePathResolutionFollowLinks(SftpConstants.SSH_FXP_STAT, "", p); + LinkOption[] options = accessor.resolveFileAccessLinkOptions( +- this, p, SftpConstants.SSH_FXP_STAT, "", true); +- return resolveFileAttributes(p, flags, options); ++ this, p, SftpConstants.SSH_FXP_STAT, "", followLinks); ++ return resolveFileAttributes(p, flags, !followLinks, options); + } + + protected void doRealPath(Buffer buffer, int id) throws IOException { ++ // do things here. + String path = buffer.getString(); + boolean debugEnabled = log.isDebugEnabled(); + ServerSession session = getServerSession(); +@@ -1467,7 +1471,6 @@ protected void doRealPath(Buffer buffer, int id) throws IOException { + result = doRealPathV6(id, path, extraPaths, p, options); + + p = result.getKey(); +- options = getPathResolutionLinkOption(SftpConstants.SSH_FXP_REALPATH, "", p); + Boolean status = result.getValue(); + switch (control) { + case SftpConstants.SSH_FXP_REALPATH_STAT_IF: +@@ -1557,14 +1560,14 @@ protected SimpleImmutableEntry validateRealPath( + int id, String path, Path f, LinkOption... options) + throws IOException { + Path p = normalize(f); +- Boolean status = IoUtils.checkFileExists(p, options); ++ Boolean status = IoUtils.checkFileExistsAnySymlinks(p, !IoUtils.followLinks(options)); + return new SimpleImmutableEntry<>(p, status); + } + + protected void doRemoveDirectory(Buffer buffer, int id) throws IOException { + String path = buffer.getString(); + try { +- doRemoveDirectory(id, path, false); ++ doRemoveDirectory(id, path); + } catch (IOException | RuntimeException e) { + sendStatus(prepareReply(buffer), id, e, + SftpConstants.SSH_FXP_RMDIR, path); +@@ -1574,15 +1577,23 @@ protected void doRemoveDirectory(Buffer buffer, int id) throws IOException { + sendStatus(prepareReply(buffer), id, SftpConstants.SSH_FX_OK, ""); + } + +- protected void doRemoveDirectory(int id, String path, boolean followLinks) throws IOException { ++ protected void doRemoveDirectory(int id, String path) throws IOException { + Path p = resolveFile(path); + if (log.isDebugEnabled()) { + log.debug("doRemoveDirectory({})[id={}] SSH_FXP_RMDIR (path={})[{}]", getServerSession(), id, path, p); + } + + SftpFileSystemAccessor accessor = getFileSystemAccessor(); ++ ++ final boolean followLinks = resolvePathResolutionFollowLinks(SftpConstants.SSH_FXP_RMDIR, "", p); ++ Boolean symlinkCheck = validateParentExistWithNoSymlinksIfNeverFollowSymlinks(p, !followLinks); ++ if (!Boolean.TRUE.equals(symlinkCheck)) { ++ throw new AccessDeniedException(p.toString(), p.toString(), ++ "Parent directories do not exist ore are prohibited symlinks"); ++ } ++ + LinkOption[] options = accessor.resolveFileAccessLinkOptions( +- this, p, SftpConstants.SSH_FXP_RMDIR, "", followLinks); ++ this, p, SftpConstants.SSH_FXP_RMDIR, "", false); + if (Files.isDirectory(p, options)) { + doRemove(id, p, true); + } else { +@@ -1618,7 +1629,7 @@ protected void doMakeDirectory(Buffer buffer, int id) throws IOException { + String path = buffer.getString(); + Map attrs = readAttrs(buffer); + try { +- doMakeDirectory(id, path, attrs, false); ++ doMakeDirectory(id, path, attrs); + } catch (IOException | RuntimeException e) { + sendStatus(prepareReply(buffer), id, e, + SftpConstants.SSH_FXP_MKDIR, path, attrs); +@@ -1629,7 +1640,7 @@ protected void doMakeDirectory(Buffer buffer, int id) throws IOException { + } + + protected void doMakeDirectory( +- int id, String path, Map attrs, boolean followLinks) ++ int id, String path, Map attrs) + throws IOException { + Path resolvedPath = resolveFile(path); + ServerSession session = getServerSession(); +@@ -1640,14 +1651,21 @@ protected void doMakeDirectory( + + SftpFileSystemAccessor accessor = getFileSystemAccessor(); + LinkOption[] options = accessor.resolveFileAccessLinkOptions( +- this, resolvedPath, SftpConstants.SSH_FXP_MKDIR, "", followLinks); ++ this, resolvedPath, SftpConstants.SSH_FXP_MKDIR, "", false); ++ final boolean followLinks = resolvePathResolutionFollowLinks(SftpConstants.SSH_FXP_MKDIR, "", resolvedPath); + SftpPathImpl.withAttributeCache(resolvedPath, p -> { +- Boolean status = IoUtils.checkFileExists(p, options); +- if (status == null) { ++ Boolean symlinkCheck = validateParentExistWithNoSymlinksIfNeverFollowSymlinks(p, !followLinks); ++ if (!Boolean.TRUE.equals(symlinkCheck)) { ++ throw new AccessDeniedException(p.toString(), p.toString(), ++ "Parent directories do not exist ore are prohibited symlinks"); ++ } ++ ++ Boolean fileExists = IoUtils.checkFileExists(p, options); ++ if (fileExists == null) { + throw new AccessDeniedException(p.toString(), p.toString(), "Cannot validate make-directory existence"); + } + +- if (status) { ++ if (fileExists) { + if (Files.isDirectory(p, options)) { + throw new FileAlreadyExistsException(p.toString(), p.toString(), "Target directory already exists"); + } else { +@@ -1661,7 +1679,6 @@ protected void doMakeDirectory( + listener.creating(session, resolvedPath, attrs); + try { + accessor.createDirectory(this, resolvedPath); +- followLinks = resolvePathResolutionFollowLinks(SftpConstants.SSH_FXP_MKDIR, "", resolvedPath); + doSetAttributes(SftpConstants.SSH_FXP_MKDIR, "", resolvedPath, attrs, followLinks); + } catch (IOException | RuntimeException | Error e) { + listener.created(session, resolvedPath, attrs, e); +@@ -1676,7 +1693,7 @@ protected void doRemove(Buffer buffer, int id) throws IOException { + /* + * If 'filename' is a symbolic link, the link is removed, not the file it points to. + */ +- doRemoveFile(id, path, false); ++ doRemoveFile(id, path); + } catch (IOException | RuntimeException e) { + sendStatus(prepareReply(buffer), id, e, SftpConstants.SSH_FXP_REMOVE, path); + return; +@@ -1685,17 +1702,20 @@ protected void doRemove(Buffer buffer, int id) throws IOException { + sendStatus(prepareReply(buffer), id, SftpConstants.SSH_FX_OK, ""); + } + +- protected void doRemoveFile(int id, String path, boolean followLinks) throws IOException { ++ protected void doRemoveFile(int id, String path) throws IOException { + Path resolvedPath = resolveFile(path); + if (log.isDebugEnabled()) { + log.debug("doRemoveFile({})[id={}] SSH_FXP_REMOVE (path={}[{}])", getServerSession(), id, path, resolvedPath); + } ++ // whether to follow links in the dir up to the final file ++ boolean followLinks = resolvePathResolutionFollowLinks(SftpConstants.SSH_FXP_REMOVE, "", resolvedPath); + + SftpFileSystemAccessor accessor = getFileSystemAccessor(); ++ // never resolve links in the final path to remove as we want to remove the symlink, not the target + LinkOption[] options = accessor.resolveFileAccessLinkOptions( +- this, resolvedPath, SftpConstants.SSH_FXP_REMOVE, "", followLinks); ++ this, resolvedPath, SftpConstants.SSH_FXP_REMOVE, "", false); + SftpPathImpl.withAttributeCache(resolvedPath, p -> { +- Boolean status = IoUtils.checkFileExists(p, options); ++ Boolean status = checkSymlinkState(p, followLinks, options); + if (status == null) { + throw signalRemovalPreConditionFailure(id, path, p, + new AccessDeniedException(p.toString(), p.toString(), "Cannot determine existence of remove candidate"), +@@ -2293,8 +2313,9 @@ protected void writeDirEntry( + int id, DirectoryHandle dir, Map entries, Buffer buffer, + int index, Path f, String shortName, LinkOption... options) + throws IOException { ++ boolean followLinks = resolvePathResolutionFollowLinks(SftpConstants.SSH_FXP_READDIR, "", f); + Map attrs = resolveFileAttributes( +- f, SftpConstants.SSH_FILEXFER_ATTR_ALL, options); ++ f, SftpConstants.SSH_FILEXFER_ATTR_ALL, !followLinks, options); + entries.put(shortName, f); + + SftpFileSystemAccessor accessor = getFileSystemAccessor(); +@@ -2392,10 +2413,10 @@ protected String getShortName(Path f) throws IOException { + } + + protected NavigableMap resolveFileAttributes( +- Path path, int flags, LinkOption... options) ++ Path path, int flags, boolean neverFollowSymLinks, LinkOption... options) + throws IOException { + return SftpPathImpl.withAttributeCache(path, file -> { +- Boolean status = IoUtils.checkFileExists(file, options); ++ Boolean status = checkSymlinkState(file, neverFollowSymLinks, options); + if (status == null) { + return handleUnknownStatusFileAttributes(file, flags, options); + } else if (!status) { +@@ -2406,7 +2427,31 @@ protected NavigableMap resolveFileAttributes( + }); + } + +- protected void writeAttrs(Buffer buffer, Map attributes) throws IOException { ++ /** ++ * A utility function to validate that the directories leading up to a file are not symlinks ++ * ++ * @param path the file to check for symlink presence ++ * @param neverFollowSymLinks whether to never follow symlinks in the parent paths ++ * @param options whether the file itself can be a symlink ++ * @return whether there are symlinks in the path to this file, or null if unknown ++ */ ++ public Boolean checkSymlinkState(Path path, boolean neverFollowSymLinks, LinkOption[] options) { ++ Boolean status = validateParentExistWithNoSymlinksIfNeverFollowSymlinks(path, neverFollowSymLinks); ++ if (!Boolean.FALSE.equals(status)) { ++ status = IoUtils.checkFileExists(path, options); ++ } ++ return status; ++ } ++ ++ public Boolean validateParentExistWithNoSymlinksIfNeverFollowSymlinks(Path path, boolean neverFollowSymLinks) { ++ Boolean status = true; ++ if (neverFollowSymLinks && path.getParent() != null) { ++ status = IoUtils.checkFileExistsAnySymlinks(path.getParent(), true); ++ } ++ return status; ++ } ++ ++ protected void writeAttrs(Buffer buffer, Map attributes) { + SftpHelper.writeAttrs(buffer, getVersion(), attributes); + } + +@@ -2653,7 +2698,11 @@ protected void setFileAttributes( + case IoUtils.SIZE_VIEW_ATTR: { + long newSize = ((Number) value).longValue(); + SftpFileSystemAccessor accessor = getFileSystemAccessor(); +- Set openOptions = EnumSet.of(StandardOpenOption.WRITE); ++ Set openOptions = new HashSet<>(); ++ openOptions.add(StandardOpenOption.WRITE); ++ if (!IoUtils.followLinks(options)) { ++ openOptions.add(LinkOption.NOFOLLOW_LINKS); ++ } + try (SeekableByteChannel channel = accessor.openFile(this, null, file, null, openOptions)) { + channel.truncate(newSize); + accessor.closeFile(this, null, file, null, channel, openOptions); +diff --git a/sshd-sftp/src/main/java/org/apache/sshd/sftp/server/SftpFileSystemAccessor.java b/sshd-sftp/src/main/java/org/apache/sshd/sftp/server/SftpFileSystemAccessor.java +index 6d7c18fc1..2f6ab0553 100644 +--- a/sshd-sftp/src/main/java/org/apache/sshd/sftp/server/SftpFileSystemAccessor.java ++++ b/sshd-sftp/src/main/java/org/apache/sshd/sftp/server/SftpFileSystemAccessor.java +@@ -19,20 +19,14 @@ + + package org.apache.sshd.sftp.server; + ++import java.io.FileNotFoundException; + import java.io.IOException; + import java.io.StreamCorruptedException; + import java.nio.channels.Channel; + import java.nio.channels.FileChannel; + import java.nio.channels.FileLock; + import java.nio.channels.SeekableByteChannel; +-import java.nio.file.CopyOption; +-import java.nio.file.DirectoryStream; +-import java.nio.file.FileSystem; +-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; ++import java.nio.file.*; + import java.nio.file.attribute.AclEntry; + import java.nio.file.attribute.AclFileAttributeView; + import java.nio.file.attribute.FileAttribute; +@@ -44,8 +38,10 @@ + import java.nio.file.attribute.UserPrincipalLookupService; + import java.nio.file.attribute.UserPrincipalNotFoundException; + import java.security.Principal; ++import java.util.Arrays; + import java.util.Collection; + import java.util.Collections; ++import java.util.HashSet; + import java.util.List; + import java.util.Map; + import java.util.NavigableMap; +@@ -214,7 +210,11 @@ default SeekableByteChannel openFile( + attrs = IoUtils.EMPTY_FILE_ATTRIBUTES; + } + +- return FileChannel.open(file, options, attrs); ++ // Don't use Set contains as this can fail with TreeSet ++ if (!Arrays.asList(options.toArray(new OpenOption[0])).contains(LinkOption.NOFOLLOW_LINKS)) { ++ return FileChannel.open(file, options, attrs); ++ } ++ return seekableByteChannelNoLinkFollow(file, options, attrs); + } + + /** +@@ -317,9 +317,12 @@ default void closeFile( + * @throws IOException If failed to open + */ + default DirectoryStream openDirectory( +- SftpSubsystemProxy subsystem, DirectoryHandle dirHandle, Path dir, String handle) ++ SftpSubsystemProxy subsystem, DirectoryHandle dirHandle, Path dir, String handle, LinkOption... linkOptions) + throws IOException { +- return Files.newDirectoryStream(dir); ++ if (IoUtils.followLinks(linkOptions)) { ++ return Files.newDirectoryStream(dir); ++ } ++ return secureResolveDirectoryStream(dir); + } + + /** +@@ -523,15 +526,84 @@ default void renameFile( + default void copyFile( + SftpSubsystemProxy subsystem, Path src, Path dst, Collection opts) + throws IOException { ++ ++ if (noFollow(opts)) { ++ try (SeekableByteChannel srcFile ++ = seekableByteChannelNoLinkFollow(src, Collections.singleton(StandardOpenOption.READ))) { ++ if (!(srcFile instanceof FileChannel)) { ++ throw new UnsupportedOperationException("Host file system must return a file channel"); ++ } ++ ++ try (SeekableByteChannel dstFile = seekableByteChannelNoLinkFollow(src, new HashSet<>(Arrays ++ .asList(StandardOpenOption.WRITE, StandardOpenOption.TRUNCATE_EXISTING, StandardOpenOption.CREATE)))) { ++ ((FileChannel) srcFile).transferTo(0, srcFile.size(), dstFile); ++ } ++ } ++ return; ++ } + Files.copy(src, dst, + GenericUtils.isEmpty(opts) + ? IoUtils.EMPTY_COPY_OPTIONS + : opts.toArray(new CopyOption[opts.size()])); + } + ++ static SeekableByteChannel seekableByteChannelNoLinkFollow( ++ Path src, Set opts, FileAttribute... fileAttributes) ++ throws IOException { ++ if (src.getNameCount() < 1) { ++ // opening root directory isn't supported. ++ throw new IllegalArgumentException(); ++ } ++ Path toResolve = src.isAbsolute() ? src : src.getFileSystem().getPath(src.toString()); ++ ++ if (!Files.isDirectory(src.getParent(), LinkOption.NOFOLLOW_LINKS)) { ++ throw new FileNotFoundException(src.getParent().toString()); ++ } ++ ++ toResolve = toResolve.normalize(); ++ try (SecureDirectoryStream ds = secureResolveDirectoryStream(toResolve.getParent())) { ++ Set newOpts = new HashSet<>(opts); ++ newOpts.add(LinkOption.NOFOLLOW_LINKS); ++ return ds.newByteChannel(toResolve.getName(toResolve.getNameCount() - 1), newOpts, fileAttributes); ++ } ++ } ++ ++ static SecureDirectoryStream secureResolveDirectoryStream(Path toResolve) throws IOException { ++ toResolve = IoUtils.removeCdUpAboveRoot(toResolve); ++ DirectoryStream ds = Files.newDirectoryStream(toResolve.getRoot()); ++ for (int i = 0; i < toResolve.getNameCount(); i++) { ++ DirectoryStream dsOld = ds; ++ try { ++ ds = secure(ds).newDirectoryStream(toResolve.getName(i), LinkOption.NOFOLLOW_LINKS); ++ dsOld.close(); ++ } catch (IOException ex) { ++ ds.close(); ++ throw ex; ++ } ++ } ++ return secure(ds); ++ } ++ ++ static SecureDirectoryStream secure(DirectoryStream ds) { ++ if (ds instanceof SecureDirectoryStream) { ++ return (SecureDirectoryStream) ds; ++ } ++ // do we want to bomb? do we want a different fallback option? ++ throw new UnsupportedOperationException("FS Does not support secure directory streams."); ++ } ++ + default void removeFile( + SftpSubsystemProxy subsystem, Path path, boolean isDirectory) + throws IOException { + Files.delete(path); + } ++ ++ default boolean noFollow(Collection opts) { ++ for (Object opt : opts) { ++ if (LinkOption.NOFOLLOW_LINKS.equals(opt)) { ++ return true; ++ } ++ } ++ return false; ++ } + } +diff --git a/sshd-sftp/src/main/java/org/apache/sshd/sftp/server/SftpSubsystem.java b/sshd-sftp/src/main/java/org/apache/sshd/sftp/server/SftpSubsystem.java +index f4c12ab36..65e282a02 100644 +--- a/sshd-sftp/src/main/java/org/apache/sshd/sftp/server/SftpSubsystem.java ++++ b/sshd-sftp/src/main/java/org/apache/sshd/sftp/server/SftpSubsystem.java +@@ -686,7 +686,7 @@ protected void doReadDir(Buffer buffer, int id) throws IOException { + // the upstream server decide. + if (!(file instanceof SftpPath)) { + LinkOption[] options = getPathResolutionLinkOption(SftpConstants.SSH_FXP_READDIR, "", file); +- Boolean status = IoUtils.checkFileExists(file, options); ++ Boolean status = IoUtils.checkFileExistsAnySymlinks(file, !IoUtils.followLinks(options)); + if (status == null) { + throw new AccessDeniedException(file.toString(), file.toString(), "Cannot determine existence of read-dir"); + } +@@ -747,7 +747,7 @@ protected void doReadDir(Buffer buffer, int id) throws IOException { + @Override + protected String doOpenDir(int id, String path, Path dir, LinkOption... options) throws IOException { + SftpPathImpl.withAttributeCache(dir, p -> { +- Boolean status = IoUtils.checkFileExists(p, options); ++ Boolean status = IoUtils.checkFileExistsAnySymlinks(p, !IoUtils.followLinks(options)); + if (status == null) { + throw signalOpenFailure(id, path, p, true, + new AccessDeniedException(p.toString(), p.toString(), "Cannot determine open-dir existence")); +@@ -807,7 +807,9 @@ protected Map doFStat(int id, String handle, int flags) throws I + Path file = fileHandle.getFile(); + LinkOption[] options = accessor.resolveFileAccessLinkOptions( + this, file, SftpConstants.SSH_FXP_FSTAT, "", true); +- return resolveFileAttributes(file, flags, options); ++ ++ boolean followLinks = resolvePathResolutionFollowLinks(SftpConstants.SSH_FXP_FSTAT, handle, file); ++ return resolveFileAttributes(file, flags, followLinks, options); + } + + @Override +diff --git a/sshd-sftp/src/test/java/org/apache/sshd/sftp/client/SftpTest.java b/sshd-sftp/src/test/java/org/apache/sshd/sftp/client/SftpTest.java +index b150e3aa0..a0b408d1b 100644 +--- a/sshd-sftp/src/test/java/org/apache/sshd/sftp/client/SftpTest.java ++++ b/sshd-sftp/src/test/java/org/apache/sshd/sftp/client/SftpTest.java +@@ -383,7 +383,7 @@ private void testCannotEscapeRoot(boolean useAbsolutePath) throws Exception { + SftpClient.Attributes attrs = sftp.stat(escapePath); + fail("Unexpected escape success for path=" + escapePath + ": " + attrs); + } catch (SftpException e) { +- int expected = OsUtils.isWin32() || (!useAbsolutePath) ++ int expected = OsUtils.isWin32() && useAbsolutePath + ? SftpConstants.SSH_FX_INVALID_FILENAME + : SftpConstants.SSH_FX_NO_SUCH_FILE; + assertEquals("Mismatched status for " + escapePath, +@@ -711,10 +711,11 @@ public SeekableByteChannel openFile( + + @Override + public DirectoryStream openDirectory( +- SftpSubsystemProxy subsystem, DirectoryHandle dirHandle, Path dir, String handle) ++ SftpSubsystemProxy subsystem, DirectoryHandle dirHandle, Path dir, String handle, ++ LinkOption... linkOptions) + throws IOException { + dirHolder.set(dir); +- return SftpFileSystemAccessor.super.openDirectory(subsystem, dirHandle, dir, handle); ++ return SftpFileSystemAccessor.super.openDirectory(subsystem, dirHandle, dir, handle, linkOptions); + } + + @Override +diff --git a/sshd-sftp/src/test/java/org/apache/sshd/sftp/client/SftpVersionsTest.java b/sshd-sftp/src/test/java/org/apache/sshd/sftp/client/SftpVersionsTest.java +index d277bbc88..c18a3d7cd 100644 +--- a/sshd-sftp/src/test/java/org/apache/sshd/sftp/client/SftpVersionsTest.java ++++ b/sshd-sftp/src/test/java/org/apache/sshd/sftp/client/SftpVersionsTest.java +@@ -291,9 +291,11 @@ public void testSftpACLEncodeDecode() throws Exception { + public Command createSubsystem(ChannelSession channel) throws IOException { + SftpSubsystem subsystem = new SftpSubsystem(channel, this) { + @Override +- protected NavigableMap resolveFileAttributes(Path file, int flags, LinkOption... options) ++ protected NavigableMap resolveFileAttributes( ++ Path file, int flags, boolean neverFollowSymLinks, LinkOption... options) + throws IOException { +- NavigableMap attrs = super.resolveFileAttributes(file, flags, options); ++ NavigableMap attrs ++ = super.resolveFileAttributes(file, flags, neverFollowSymLinks, options); + if (MapEntryUtils.isEmpty(attrs)) { + attrs = new TreeMap<>(String.CASE_INSENSITIVE_ORDER); + } +@@ -412,9 +414,11 @@ public void testSftpExtensionsEncodeDecode() throws Exception { + public Command createSubsystem(ChannelSession channel) throws IOException { + SftpSubsystem subsystem = new SftpSubsystem(channel, this) { + @Override +- protected NavigableMap resolveFileAttributes(Path file, int flags, LinkOption... options) ++ protected NavigableMap resolveFileAttributes( ++ Path file, int flags, boolean neverFollowLinks, LinkOption... options) + throws IOException { +- NavigableMap attrs = super.resolveFileAttributes(file, flags, options); ++ NavigableMap attrs ++ = super.resolveFileAttributes(file, flags, neverFollowLinks, options); + if (MapEntryUtils.isEmpty(attrs)) { + attrs = new TreeMap<>(String.CASE_INSENSITIVE_ORDER); + } diff --git a/apache-sshd.spec b/apache-sshd.spec index 1dc32a2..ca045cb 100644 --- a/apache-sshd.spec +++ b/apache-sshd.spec @@ -1,13 +1,15 @@ Epoch: 1 Name: apache-sshd Version: 2.9.2 -Release: 1 +Release: 2 Summary: Apache SSHD License: ASL 2.0 and ISC URL: http://mina.apache.org/sshd-project Source0: https://archive.apache.org/dist/mina/sshd/%{version}/apache-sshd-%{version}-src.tar.gz Patch0: 0001-Avoid-optional-dependency-on-native-tomcat-APR-libra.patch Patch1: apache-sshd-javadoc.patch +# https://github.com/apache/mina-sshd/commit/c20739b43aab0f7bf2ccad982a6cb37b9d5a8a0b +Patch2: CVE-2023-35887.patch BuildRequires: maven-local mvn(junit:junit) mvn(net.i2p.crypto:eddsa) mvn(org.apache.ant:ant) BuildRequires: mvn(org.apache:apache:pom:) mvn(org.apache.felix:maven-bundle-plugin) @@ -32,9 +34,7 @@ Summary: API documentation for %{name} This package provides %{name}. %prep -%setup -q -%patch0 -p1 -%patch1 -p1 +%autosetup -p1 rm -rf sshd-core/src/main/java/org/apache/sshd/agent/unix %pom_remove_dep :spring-framework-bom %pom_remove_dep :testcontainers-bom sshd-sftp sshd-core @@ -71,6 +71,9 @@ rm -rf sshd-core/src/main/java/org/apache/sshd/agent/unix %license LICENSE.txt NOTICE.txt assembly/src/main/legal/licenses/jbcrypt.txt %changelog +* Thu Jan 11 2024 yaoxin - 1:2.9.2-2 +- Fix CVE-2023-35887 + * Mon Nov 21 2022 liangqifeng - 1:2.9.2-1 - Fix CVE-2022-45047