You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mina.apache.org by lg...@apache.org on 2015/07/27 10:32:55 UTC
[1/2] mina-sshd git commit: [SSHD-545] Prevent OutOfMemoryError due
to malformed SFTP read request
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 <lg...@vmware.com>
Authored: Mon Jul 27 11:32:33 2015 +0300
Committer: Lyor Goldstein <lg...@vmware.com>
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();
[2/2] mina-sshd git commit: Avoid re-creating growth functions
repeatedly
Posted by lg...@apache.org.
Avoid re-creating growth functions repeatedly
Project: http://git-wip-us.apache.org/repos/asf/mina-sshd/repo
Commit: http://git-wip-us.apache.org/repos/asf/mina-sshd/commit/9521dfa2
Tree: http://git-wip-us.apache.org/repos/asf/mina-sshd/tree/9521dfa2
Diff: http://git-wip-us.apache.org/repos/asf/mina-sshd/diff/9521dfa2
Branch: refs/heads/master
Commit: 9521dfa27498ef718920a85aaf3d195243e46ee2
Parents: 3c2a466
Author: Lyor Goldstein <lg...@vmware.com>
Authored: Mon Jul 27 11:32:46 2015 +0300
Committer: Lyor Goldstein <lg...@vmware.com>
Committed: Mon Jul 27 11:32:46 2015 +0300
----------------------------------------------------------------------
.../java/org/apache/sshd/common/channel/AbstractChannel.java | 7 ++++++-
.../sshd/common/session/AbstractConnectionService.java | 7 ++++++-
.../apache/sshd/server/global/CancelTcpipForwardHandler.java | 7 ++++++-
.../org/apache/sshd/server/global/TcpipForwardHandler.java | 8 +++++++-
4 files changed, 25 insertions(+), 4 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/9521dfa2/sshd-core/src/main/java/org/apache/sshd/common/channel/AbstractChannel.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/main/java/org/apache/sshd/common/channel/AbstractChannel.java b/sshd-core/src/main/java/org/apache/sshd/common/channel/AbstractChannel.java
index 7b613f5..4c24645 100644
--- a/sshd-core/src/main/java/org/apache/sshd/common/channel/AbstractChannel.java
+++ b/sshd-core/src/main/java/org/apache/sshd/common/channel/AbstractChannel.java
@@ -59,6 +59,11 @@ public abstract class AbstractChannel
public static final long DEFAULT_CHANNEL_CLOSE_TIMEOUT = 5000;
+ /**
+ * Default growth factor function used to resize response buffers
+ */
+ public static final Int2IntFunction RESPONSE_BUFFER_GROWTH_FACTOR = Int2IntFunction.Utils.add(Byte.SIZE);
+
protected static enum GracefulState {
Opened, CloseSent, CloseReceived, Closed
}
@@ -181,7 +186,7 @@ public abstract class AbstractChannel
: SshConstants.SSH_MSG_CHANNEL_FAILURE;
buffer.clear();
// leave room for the SSH header
- buffer.ensureCapacity(5 + 1 + (Integer.SIZE / Byte.SIZE), Int2IntFunction.Utils.add(Byte.SIZE));
+ buffer.ensureCapacity(5 + 1 + (Integer.SIZE / Byte.SIZE), RESPONSE_BUFFER_GROWTH_FACTOR);
buffer.rpos(5);
buffer.wpos(5);
buffer.putByte(cmd);
http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/9521dfa2/sshd-core/src/main/java/org/apache/sshd/common/session/AbstractConnectionService.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/main/java/org/apache/sshd/common/session/AbstractConnectionService.java b/sshd-core/src/main/java/org/apache/sshd/common/session/AbstractConnectionService.java
index 48b3a0a..939d283 100644
--- a/sshd-core/src/main/java/org/apache/sshd/common/session/AbstractConnectionService.java
+++ b/sshd-core/src/main/java/org/apache/sshd/common/session/AbstractConnectionService.java
@@ -76,6 +76,11 @@ public abstract class AbstractConnectionService extends CloseableUtils.AbstractI
public static final int DEFAULT_MAX_CHANNELS = Integer.MAX_VALUE;
/**
+ * Default growth factor function used to resize response buffers
+ */
+ public static final Int2IntFunction RESPONSE_BUFFER_GROWTH_FACTOR = Int2IntFunction.Utils.add(Byte.SIZE);
+
+ /**
* Map of channels keyed by the identifier
*/
protected final Map<Integer, Channel> channels = new ConcurrentHashMap<>();
@@ -472,7 +477,7 @@ public abstract class AbstractConnectionService extends CloseableUtils.AbstractI
: SshConstants.SSH_MSG_CHANNEL_FAILURE;
buffer.clear();
// leave room for the SSH header
- buffer.ensureCapacity(5 + 1 + (Integer.SIZE / Byte.SIZE), Int2IntFunction.Utils.add(Byte.SIZE));
+ buffer.ensureCapacity(5 + 1 + (Integer.SIZE / Byte.SIZE), RESPONSE_BUFFER_GROWTH_FACTOR);
buffer.rpos(5);
buffer.wpos(5);
buffer.putByte(cmd);
http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/9521dfa2/sshd-core/src/main/java/org/apache/sshd/server/global/CancelTcpipForwardHandler.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/main/java/org/apache/sshd/server/global/CancelTcpipForwardHandler.java b/sshd-core/src/main/java/org/apache/sshd/server/global/CancelTcpipForwardHandler.java
index b68d54b..483b0cb 100644
--- a/sshd-core/src/main/java/org/apache/sshd/server/global/CancelTcpipForwardHandler.java
+++ b/sshd-core/src/main/java/org/apache/sshd/server/global/CancelTcpipForwardHandler.java
@@ -35,6 +35,11 @@ import org.apache.sshd.common.util.logging.AbstractLoggingBean;
*/
public class CancelTcpipForwardHandler extends AbstractLoggingBean implements ConnectionServiceRequestHandler {
public static final String REQUEST = "cancel-tcpip-forward";
+ /**
+ * Default growth factor function used to resize response buffers
+ */
+ public static final Int2IntFunction RESPONSE_BUFFER_GROWTH_FACTOR = Int2IntFunction.Utils.add(Byte.SIZE);
+
public static final CancelTcpipForwardHandler INSTANCE = new CancelTcpipForwardHandler();
public CancelTcpipForwardHandler() {
@@ -57,7 +62,7 @@ public class CancelTcpipForwardHandler extends AbstractLoggingBean implements Co
if (wantReply) {
buffer.clear();
// leave room for the SSH header
- buffer.ensureCapacity(5 + 1 + (Integer.SIZE / Byte.SIZE), Int2IntFunction.Utils.add(Byte.SIZE));
+ buffer.ensureCapacity(5 + 1 + (Integer.SIZE / Byte.SIZE), RESPONSE_BUFFER_GROWTH_FACTOR);
buffer.rpos(5);
buffer.wpos(5);
buffer.putByte(SshConstants.SSH_MSG_REQUEST_SUCCESS);
http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/9521dfa2/sshd-core/src/main/java/org/apache/sshd/server/global/TcpipForwardHandler.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/main/java/org/apache/sshd/server/global/TcpipForwardHandler.java b/sshd-core/src/main/java/org/apache/sshd/server/global/TcpipForwardHandler.java
index 4b0f0c4..c554f3b 100644
--- a/sshd-core/src/main/java/org/apache/sshd/server/global/TcpipForwardHandler.java
+++ b/sshd-core/src/main/java/org/apache/sshd/server/global/TcpipForwardHandler.java
@@ -35,6 +35,12 @@ import org.apache.sshd.common.util.logging.AbstractLoggingBean;
*/
public class TcpipForwardHandler extends AbstractLoggingBean implements ConnectionServiceRequestHandler {
public static final String REQUEST = "tcpip-forward";
+
+ /**
+ * Default growth factor function used to resize response buffers
+ */
+ public static final Int2IntFunction RESPONSE_BUFFER_GROWTH_FACTOR = Int2IntFunction.Utils.add(Byte.SIZE);
+
public static final TcpipForwardHandler INSTANCE = new TcpipForwardHandler();
public TcpipForwardHandler() {
@@ -57,7 +63,7 @@ public class TcpipForwardHandler extends AbstractLoggingBean implements Connecti
if (wantReply) {
buffer.clear();
// leave room for the SSH header
- buffer.ensureCapacity(5 + 1 + (Integer.SIZE / Byte.SIZE), Int2IntFunction.Utils.add(Byte.SIZE));
+ buffer.ensureCapacity(5 + 1 + (Integer.SIZE / Byte.SIZE), RESPONSE_BUFFER_GROWTH_FACTOR);
buffer.rpos(5);
buffer.wpos(5);
buffer.putByte(SshConstants.SSH_MSG_REQUEST_SUCCESS);