Repository: mina-sshd Updated Branches: refs/heads/master 622889259 -> 9521dfa27
[SSHD-545] Prevent OutOfMemoryError due to malformed SFTP read request Project: http://git-wip-us.apache.org/repos/asf/mina-sshd/repo Commit: http://git-wip-us.apache.org/repos/asf/mina-sshd/commit/3c2a4666 Tree: http://git-wip-us.apache.org/repos/asf/mina-sshd/tree/3c2a4666 Diff: http://git-wip-us.apache.org/repos/asf/mina-sshd/diff/3c2a4666 Branch: refs/heads/master Commit: 3c2a4666bf3c7a2713c7915c251d2c1eb2e6ab76 Parents: 6228892 Author: Lyor Goldstein <[email protected]> Authored: Mon Jul 27 11:32:33 2015 +0300 Committer: Lyor Goldstein <[email protected]> Committed: Mon Jul 27 11:32:33 2015 +0300 ---------------------------------------------------------------------- .../server/subsystem/sftp/SftpSubsystem.java | 17 +++++-- .../sshd/client/subsystem/sftp/SftpTest.java | 52 +++++++++++++++++++- 2 files changed, 64 insertions(+), 5 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/3c2a4666/sshd-core/src/main/java/org/apache/sshd/server/subsystem/sftp/SftpSubsystem.java ---------------------------------------------------------------------- diff --git a/sshd-core/src/main/java/org/apache/sshd/server/subsystem/sftp/SftpSubsystem.java b/sshd-core/src/main/java/org/apache/sshd/server/subsystem/sftp/SftpSubsystem.java index 54acc9a..5d5092f 100644 --- a/sshd-core/src/main/java/org/apache/sshd/server/subsystem/sftp/SftpSubsystem.java +++ b/sshd-core/src/main/java/org/apache/sshd/server/subsystem/sftp/SftpSubsystem.java @@ -217,7 +217,8 @@ public class SftpSubsystem extends AbstractLoggingBean implements Command, Runna /** * Force the use of a max. packet length - especially for {@link #doReadDir(Buffer, int)} - * + * and {@link #doRead(Buffer, int)} methods + * * @see #DEFAULT_MAX_PACKET_LENGTH */ public static final String MAX_PACKET_LENGTH_PROP = "sftp-max-packet-length"; @@ -1884,10 +1885,20 @@ public class SftpSubsystem extends AbstractLoggingBean implements Command, Runna protected void doRead(Buffer buffer, int id) throws IOException { String handle = buffer.getString(); long offset = buffer.getLong(); - int readLen = buffer.getInt(); + int requestedLength = buffer.getInt(); + int maxAllowed = FactoryManagerUtils.getIntProperty(session, MAX_PACKET_LENGTH_PROP, DEFAULT_MAX_PACKET_LENGTH); + int readLen = Math.min(requestedLength, maxAllowed); + + if (log.isTraceEnabled()) { + log.trace("doRead({})[offset={}] - req.={}, max.={}, effective={}", + handle, offset, requestedLength, maxAllowed, readLen); + } + try { + ValidateUtils.checkTrue(readLen >= 0, "Illegal requested read length: %d", readLen); + buffer.clear(); - buffer.ensureCapacity(readLen + Long.SIZE, Int2IntFunction.IDENTITY); + buffer.ensureCapacity(readLen + Long.SIZE /* the header */, Int2IntFunction.IDENTITY); buffer.putByte((byte) SSH_FXP_DATA); buffer.putInt(id); http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/3c2a4666/sshd-core/src/test/java/org/apache/sshd/client/subsystem/sftp/SftpTest.java ---------------------------------------------------------------------- diff --git a/sshd-core/src/test/java/org/apache/sshd/client/subsystem/sftp/SftpTest.java b/sshd-core/src/test/java/org/apache/sshd/client/subsystem/sftp/SftpTest.java index c5fb7c4..6fd49d8 100644 --- a/sshd-core/src/test/java/org/apache/sshd/client/subsystem/sftp/SftpTest.java +++ b/sshd-core/src/test/java/org/apache/sshd/client/subsystem/sftp/SftpTest.java @@ -47,10 +47,13 @@ import java.util.concurrent.atomic.AtomicInteger; import org.apache.sshd.client.SshClient; import org.apache.sshd.client.session.ClientSession; +import org.apache.sshd.client.subsystem.sftp.SftpClient.CloseableHandle; +import org.apache.sshd.client.subsystem.sftp.SftpClient.OpenMode; import org.apache.sshd.client.subsystem.sftp.extensions.BuiltinSftpClientExtensions; import org.apache.sshd.client.subsystem.sftp.extensions.SftpClientExtension; import org.apache.sshd.common.Factory; import org.apache.sshd.common.FactoryManager; +import org.apache.sshd.common.FactoryManagerUtils; import org.apache.sshd.common.file.FileSystemFactory; import org.apache.sshd.common.random.Random; import org.apache.sshd.common.session.Session; @@ -113,6 +116,52 @@ public class SftpTest extends AbstractSftpClientTestSupport { Thread.sleep(5 * 60000); } + @Test // see SSHD-545 + public void testReadBufferLimit() throws Exception { + Path targetPath = detectTargetFolder().toPath(); + Path parentPath = targetPath.getParent(); + Path lclSftp = Utils.resolve(targetPath, SftpConstants.SFTP_SUBSYSTEM_NAME, getClass().getSimpleName(), getCurrentTestName()); + Path testFile = assertHierarchyTargetFolderExists(lclSftp).resolve("file.txt"); + byte[] expected = new byte[1024]; + + Factory<? extends Random> factory = sshd.getRandomFactory(); + Random rnd = factory.create(); + rnd.fill(expected); + Files.write(testFile, expected); + + try (SshClient client = SshClient.setUpDefaultClient()) { + client.start(); + + try (ClientSession session = client.connect(getCurrentTestName(), "localhost", port).verify(7L, TimeUnit.SECONDS).getSession()) { + session.addPasswordIdentity(getCurrentTestName()); + session.auth().verify(5L, TimeUnit.SECONDS); + + try (SftpClient sftp = session.createSftpClient()) { + String file = Utils.resolveRelativeRemotePath(parentPath, testFile); + byte[] actual = new byte[expected.length]; + int maxAllowed = actual.length / 4; + // allow less than actual + FactoryManagerUtils.updateProperty(sshd, SftpSubsystem.MAX_PACKET_LENGTH_PROP, maxAllowed); + try(CloseableHandle handle = sftp.open(file, OpenMode.Read)) { + int readLen = sftp.read(handle, 0L, actual); + assertEquals("Mismatched read len", maxAllowed, readLen); + + for (int index = 0; index < readLen; index++) { + byte expByte = expected[index], actByte = actual[index]; + if (expByte != actByte) { + fail("Mismatched values at index=" + index + + ": expected=0x" + Integer.toHexString(expByte & 0xFF) + + ", actual=0x" + Integer.toHexString(actByte & 0xFF)); + } + } + } + } + } finally { + client.stop(); + } + } + } + @Test // see extra fix for SSHD-538 public void testNavigateBeyondRootFolder() throws Exception { Path rootLocation = Paths.get(OsUtils.isUNIX() ? "/" : "C:\\"); @@ -179,11 +228,10 @@ public class SftpTest extends AbstractSftpClientTestSupport { Path targetPath = detectTargetFolder().toPath(); Path parentPath = targetPath.getParent(); Path lclSftp = Utils.resolve(targetPath, SftpConstants.SFTP_SUBSYSTEM_NAME, getClass().getSimpleName(), getCurrentTestName()); - Path testFile = assertHierarchyTargetFolderExists(lclSftp).resolve("file.txt"); - String file = Utils.resolveRelativeRemotePath(parentPath, testFile); String[] comps = GenericUtils.split(file, '/'); + try (SshClient client = SshClient.setUpDefaultClient()) { client.start();
