lgoldstein commented on code in PR #362: URL: https://github.com/apache/mina-sshd/pull/362#discussion_r1172862559
########## 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 <a href="mailto:dev@mina.apache.org";>Apache MINA SSHD Project</a> + */ +class RootedDirectoryStream implements DirectoryStream<Path> { Review Comment: I think this class (and its constructor) should be *public* since we want to give our users the flexibility to implement behavior override if they should feel like it. ########## sshd-common/src/main/java/org/apache/sshd/common/file/root/RootedFileSystemUtils.java: ########## @@ -0,0 +1,49 @@ +/* + * 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 + } + + static void validateSafeRelativeSymlink(Path target) { Review Comment: I believe the method should also be *public* ########## 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 <a href="mailto:dev@mina.apache.org";>Apache MINA SSHD Project</a> + */ +class RootedSecureDirectoryStream extends RootedDirectoryStream implements SecureDirectoryStream<Path> { + + RootedSecureDirectoryStream(RootedFileSystem rfs, SecureDirectoryStream<Path> delegate) { + super(rfs, delegate); + } + + @Override + public SecureDirectoryStream<Path> newDirectoryStream(Path path, LinkOption... options) throws IOException { + return new RootedSecureDirectoryStream(rfs, delegate().newDirectoryStream(fixPath(path), options)); + } + + private Path fixPath(Path p) { Review Comment: Should be *protected* in order to allow users to override the behavior if they should wish to do so. ########## 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 <a href="mailto:dev@mina.apache.org";>Apache MINA SSHD Project</a> + */ +class RootedSecureDirectoryStream extends RootedDirectoryStream implements SecureDirectoryStream<Path> { Review Comment: See previous comments regarding *public* visibility... ########## sshd-common/src/main/java/org/apache/sshd/common/file/util/BaseFileSystem.java: ########## @@ -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 + */ + private String handleWindowsSeparator(String name) { Review Comment: Should be *protected* in order to allow users to override the behavior if they should wish to do so. ########## sshd-common/src/main/java/org/apache/sshd/common/util/io/IoUtils.java: ########## @@ -415,6 +442,19 @@ public static Boolean checkFileExists(Path path, LinkOption... options) { } } + private static Path getPartialPath(Path path, int partsToExtract) { Review Comment: Should be *public* in case users want/need to use it ########## sshd-common/src/main/java/org/apache/sshd/common/util/io/IoUtils.java: ########## @@ -637,4 +677,75 @@ public static List<String> 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<String> 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<String> sanitized = removeExtraCdUps(toSanitize); + return buildPath(toSanitize.getRoot(), toSanitize.getFileSystem(), sanitized); + } + + private static List<String> removeExtraCdUps(Path toResolve) { + List<String> 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; + } + + private static Path buildPath(Path root, FileSystem fs, List<String> namesList) { Review Comment: Should be *public* in case users want/need to use it ########## sshd-sftp/src/main/java/org/apache/sshd/sftp/server/AbstractSftpSubsystemHelper.java: ########## @@ -2406,6 +2428,31 @@ protected NavigableMap<String, Object> resolveFileAttributes( }); } + /** + * A utility function to validate that the directories leading up to a file are not symlinks when they are not + * supposed to be. + * + * @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 the status of the file + */ + private static 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; + } + + private static Boolean validateParentExistWithNoSymlinksIfNeverFollowSymlinks(Path path, boolean neverFollowSymLinks) { Review Comment: Should be *public* in case users want/need to use it ########## sshd-sftp/src/main/java/org/apache/sshd/sftp/server/AbstractSftpSubsystemHelper.java: ########## @@ -2406,6 +2428,31 @@ protected NavigableMap<String, Object> resolveFileAttributes( }); } + /** + * A utility function to validate that the directories leading up to a file are not symlinks when they are not + * supposed to be. + * + * @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 the status of the file + */ + private static Boolean checkSymlinkState(Path path, boolean neverFollowSymLinks, LinkOption[] options) { Review Comment: Should be *public* in case users want/need to use it ########## sshd-common/src/main/java/org/apache/sshd/common/util/io/IoUtils.java: ########## @@ -637,4 +677,75 @@ public static List<String> 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<String> 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<String> sanitized = removeExtraCdUps(toSanitize); + return buildPath(toSanitize.getRoot(), toSanitize.getFileSystem(), sanitized); + } + + private static List<String> removeExtraCdUps(Path toResolve) { + List<String> 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; + } + + private static Path buildPath(Path root, FileSystem fs, List<String> namesList) { + if (namesList.isEmpty()) { + return root == null ? fs.getPath(".") : root; + } + + Path cleanedPathToResolve = buildPath(fs, namesList); + return root.resolve(cleanedPathToResolve); + } + + private static Path buildPath(FileSystem fs, List<String> namesList) { Review Comment: Should be *public* in case users want/need to use it -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@mina.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@mina.apache.org For additional commands, e-mail: dev-h...@mina.apache.org
