diff --git a/src/main/java/net/schmizz/sshj/xfer/FileSystemFile.java b/src/main/java/net/schmizz/sshj/xfer/FileSystemFile.java index 642fc11f..83d5a261 100644 --- a/src/main/java/net/schmizz/sshj/xfer/FileSystemFile.java +++ b/src/main/java/net/schmizz/sshj/xfer/FileSystemFile.java @@ -21,6 +21,7 @@ import org.slf4j.LoggerFactory; import java.io.*; import java.util.ArrayList; import java.util.List; +import java.util.Stack; public class FileSystemFile implements LocalSourceFile, LocalDestFile { @@ -83,8 +84,9 @@ public class FileSystemFile } }); - if (childFiles == null) + if (childFiles == null) { throw new IOException("Error listing files in directory: " + this); + } final List children = new ArrayList(); for (File f : childFiles) { @@ -113,12 +115,13 @@ public class FileSystemFile @Override public int getPermissions() throws IOException { - if (isDirectory()) + if (isDirectory()) { return 0755; - else if (isFile()) + } else if (isFile()) { return 0644; - else + } else { throw new IOException("Unsupported file type"); + } } @Override @@ -130,8 +133,9 @@ public class FileSystemFile @Override public void setLastModifiedTime(long t) throws IOException { - if (!file.setLastModified(t * 1000)) + if (!file.setLastModified(t * 1000)) { log.warn("Could not set last modified time for {} to {}", file, t); + } } @Override @@ -143,22 +147,41 @@ public class FileSystemFile !(FilePermission.OTH_W.isIn(perms) || FilePermission.GRP_W.isIn(perms))); final boolean x = file.setExecutable(FilePermission.USR_X.isIn(perms), !(FilePermission.OTH_X.isIn(perms) || FilePermission.GRP_X.isIn(perms))); - if (!(r && w && x)) + if (!(r && w && x)) { log.warn("Could not set permissions for {} to {}", file, Integer.toString(perms, 16)); + } } @Override public FileSystemFile getChild(String name) { + validateIsChildPath(name); return new FileSystemFile(new File(file, name)); } + private void validateIsChildPath(String name) { + String[] split = name.split("/"); + Stack s = new Stack(); + for (String component : split) { + if (component == null || component.isEmpty() || component.equals(".")) { + continue; + } else if (component.equals("..") && !s.isEmpty()) { + s.pop(); + continue; + } else if (component.equals("..")) { + throw new IllegalArgumentException("Cannot traverse higher than " + file + " to get child " + name); + } + s.push(component); + } + } + @Override public FileSystemFile getTargetFile(String filename) throws IOException { FileSystemFile f = this; - if (f.isDirectory()) + if (f.isDirectory()) { f = f.getChild(filename); + } if (!f.getFile().exists()) { if (!f.getFile().createNewFile()) @@ -174,12 +197,15 @@ public class FileSystemFile throws IOException { FileSystemFile f = this; - if (f.getFile().exists()) + if (f.getFile().exists()) { if (f.isDirectory()) { - if (!f.getName().equals(dirname)) + if (!f.getName().equals(dirname)) { f = f.getChild(dirname); - } else + } + } else { throw new IOException(f + " - already exists as a file; directory required"); + } + } if (!f.getFile().exists() && !f.getFile().mkdir()) throw new IOException("Failed to create directory: " + f); diff --git a/src/main/java/net/schmizz/sshj/xfer/scp/SCPDownloadClient.java b/src/main/java/net/schmizz/sshj/xfer/scp/SCPDownloadClient.java index fd25144d..f9779497 100644 --- a/src/main/java/net/schmizz/sshj/xfer/scp/SCPDownloadClient.java +++ b/src/main/java/net/schmizz/sshj/xfer/scp/SCPDownloadClient.java @@ -75,9 +75,9 @@ public class SCPDownloadClient extends AbstractSCPClient { engine.signal("Start status OK"); String msg = engine.readMessage(); - do + do { process(engine.getTransferListener(), null, msg, targetFile); - while (!(msg = engine.readMessage()).isEmpty()); + } while (!(msg = engine.readMessage()).isEmpty()); } private long parseLong(String longString, String valType) @@ -93,15 +93,17 @@ public class SCPDownloadClient extends AbstractSCPClient { private int parsePermissions(String cmd) throws SCPException { - if (cmd.length() != 5) + if (cmd.length() != 5) { throw new SCPException("Could not parse permissions from `" + cmd + "`"); + } return Integer.parseInt(cmd.substring(1), 8); } private boolean process(TransferListener listener, String bufferedTMsg, String msg, LocalDestFile f) throws IOException { - if (msg.length() < 1) + if (msg.length() < 1) { throw new SCPException("Could not parse message `" + msg + "`"); + } switch (msg.charAt(0)) { @@ -139,8 +141,9 @@ public class SCPDownloadClient extends AbstractSCPClient { final List dMsgParts = tokenize(dMsg, 3, true); // D 0 final long length = parseLong(dMsgParts.get(1), "dir length"); final String dirname = dMsgParts.get(2); - if (length != 0) + if (length != 0) { throw new IOException("Remote SCP command sent strange directory length: " + length); + } final TransferListener dirListener = listener.directory(dirname); { @@ -186,9 +189,9 @@ public class SCPDownloadClient extends AbstractSCPClient { private static List tokenize(String msg, int totalParts, boolean consolidateTail) throws IOException { List parts = Arrays.asList(msg.split(" ")); - if (parts.size() < totalParts || - (!consolidateTail && parts.size() != totalParts)) + if (parts.size() < totalParts || (!consolidateTail && parts.size() != totalParts)) { throw new IOException("Could not parse message received from remote SCP: " + msg); + } if (consolidateTail && totalParts < parts.size()) { final StringBuilder sb = new StringBuilder(parts.get(totalParts - 1)); diff --git a/src/test/groovy/net/schmizz/sshj/xfer/FileSystemFileSpec.groovy b/src/test/groovy/net/schmizz/sshj/xfer/FileSystemFileSpec.groovy new file mode 100644 index 00000000..c9708a26 --- /dev/null +++ b/src/test/groovy/net/schmizz/sshj/xfer/FileSystemFileSpec.groovy @@ -0,0 +1,54 @@ +/* + * Copyright (C)2009 - SSHJ Contributors + * + * Licensed 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 net.schmizz.sshj.xfer + +import spock.lang.Specification + +class FileSystemFileSpec extends Specification { + + def "should get child path"() { + given: + def file = new FileSystemFile("foo") + + when: + def child = file.getChild("bar") + + then: + child.getName() == "bar" + } + + def "should not traverse higher than original path when getChild is called"() { + given: + def file = new FileSystemFile("foo") + + when: + file.getChild("bar/.././foo/../../") + + then: + thrown(IllegalArgumentException.class) + } + + def "should ignore double slash (empty path component)"() { + given: + def file = new FileSystemFile("foo") + + when: + def child = file.getChild("bar//etc/passwd") + + then: + child.getFile().getPath() endsWith "foo/bar/etc/passwd" + } +}