[SSHD-699] Ignore malformed SSH_MSG_IGNORE/DEBUG messages
Project: http://git-wip-us.apache.org/repos/asf/mina-sshd/repo Commit: http://git-wip-us.apache.org/repos/asf/mina-sshd/commit/110ee570 Tree: http://git-wip-us.apache.org/repos/asf/mina-sshd/tree/110ee570 Diff: http://git-wip-us.apache.org/repos/asf/mina-sshd/diff/110ee570 Branch: refs/heads/master Commit: 110ee5702ac352331938f7bdf899ea3c7f369917 Parents: f560959 Author: Lyor Goldstein <[email protected]> Authored: Fri Sep 16 18:16:26 2016 +0300 Committer: Lyor Goldstein <[email protected]> Committed: Fri Sep 16 18:16:26 2016 +0300 ---------------------------------------------------------------------- .../common/session/helpers/AbstractSession.java | 18 ++++- .../apache/sshd/common/util/buffer/Buffer.java | 66 +++++++++++++++++ .../common/util/buffer/ByteArrayBuffer.java | 7 +- .../session/helpers/AbstractSessionTest.java | 77 ++++++++++++++++++++ 4 files changed, 166 insertions(+), 2 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/110ee570/sshd-core/src/main/java/org/apache/sshd/common/session/helpers/AbstractSession.java ---------------------------------------------------------------------- diff --git a/sshd-core/src/main/java/org/apache/sshd/common/session/helpers/AbstractSession.java b/sshd-core/src/main/java/org/apache/sshd/common/session/helpers/AbstractSession.java index 290ce96..d101e37 100644 --- a/sshd-core/src/main/java/org/apache/sshd/common/session/helpers/AbstractSession.java +++ b/sshd-core/src/main/java/org/apache/sshd/common/session/helpers/AbstractSession.java @@ -606,6 +606,14 @@ public abstract class AbstractSession extends AbstractKexFactoryManager implemen } protected void handleIgnore(Buffer buffer) throws Exception { + // malformed ignore message - ignore (even though we don't have to, but we can be tolerant in this case) + if (!buffer.isValidMessageStructure(byte[].class)) { + if (log.isTraceEnabled()) { + log.trace("handleDebug({}) ignore malformed message", this); + } + return; + } + ReservedSessionMessagesHandler handler = resolveReservedSessionMessagesHandler(); handler.handleIgnoreMessage(this, buffer); } @@ -626,7 +634,7 @@ public abstract class AbstractSession extends AbstractKexFactoryManager implemen @Override public IoWriteFuture sendDebugMessage(boolean display, Object msg, String lang) throws IOException { - String text = Objects.toString(msg); + String text = Objects.toString(msg, ""); lang = (lang == null) ? "" : lang; Buffer buffer = createBuffer(SshConstants.SSH_MSG_DEBUG, @@ -638,6 +646,14 @@ public abstract class AbstractSession extends AbstractKexFactoryManager implemen } protected void handleDebug(Buffer buffer) throws Exception { + // malformed ignore message - ignore (even though we don't have to, but we can be tolerant in this case) + if (!buffer.isValidMessageStructure(boolean.class, String.class, String.class)) { + if (log.isTraceEnabled()) { + log.trace("handleDebug({}) ignore malformed message", this); + } + return; + } + ReservedSessionMessagesHandler handler = resolveReservedSessionMessagesHandler(); handler.handleDebugMessage(this, buffer); } http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/110ee570/sshd-core/src/main/java/org/apache/sshd/common/util/buffer/Buffer.java ---------------------------------------------------------------------- diff --git a/sshd-core/src/main/java/org/apache/sshd/common/util/buffer/Buffer.java b/sshd-core/src/main/java/org/apache/sshd/common/util/buffer/Buffer.java index 5da403c..16a0368 100644 --- a/sshd-core/src/main/java/org/apache/sshd/common/util/buffer/Buffer.java +++ b/sshd-core/src/main/java/org/apache/sshd/common/util/buffer/Buffer.java @@ -44,6 +44,7 @@ import java.security.spec.InvalidKeySpecException; import java.security.spec.RSAPrivateCrtKeySpec; import java.security.spec.RSAPublicKeySpec; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.LinkedList; @@ -108,6 +109,71 @@ public abstract class Buffer implements Readable { public abstract void clear(); + public boolean isValidMessageStructure(Class<?>... fieldTypes) { + return isValidMessageStructure(GenericUtils.isEmpty(fieldTypes) ? Collections.emptyList() : Arrays.asList(fieldTypes)); + } + + public boolean isValidMessageStructure(Collection<Class<?>> fieldTypes) { + if (GenericUtils.isEmpty(fieldTypes)) { + return true; + } + + int remainLen = available(); + int readOffset = 0; + for (Class<?> ft : fieldTypes) { + if ((ft == boolean.class) || (ft == Boolean.class) + || (ft == byte.class) || (ft == Byte.class)) { + if (remainLen < Byte.BYTES) { + return false; + } + + remainLen -= Byte.BYTES; + readOffset += Byte.BYTES; + } else if ((ft == short.class) || (ft == Short.class)) { + if (remainLen < Short.BYTES) { + return false; + } + + remainLen -= Short.BYTES; + readOffset += Short.BYTES; + } else if ((ft == int.class) || (ft == Integer.class)) { + if (remainLen < Integer.BYTES) { + return false; + } + + remainLen -= Integer.BYTES; + readOffset += Integer.BYTES; + } else if ((ft == long.class) || (ft == Long.class)) { + if (remainLen < Long.BYTES) { + return false; + } + + remainLen -= Long.BYTES; + readOffset += Long.BYTES; + } else if ((ft == byte[].class) || (ft == String.class)) { + if (remainLen < Integer.BYTES) { + return false; + } + + copyRawBytes(readOffset, workBuf, 0, Integer.BYTES); + remainLen -= Integer.BYTES; + readOffset += Integer.BYTES; + + long length = BufferUtils.getUInt(workBuf, 0, Integer.BYTES); + if (length > remainLen) { + return false; + } + + remainLen -= (int) length; + readOffset += (int) length; + } + } + + return true; + } + + protected abstract void copyRawBytes(int offset, byte[] buf, int pos, int len); + public String toHex() { return BufferUtils.toHex(array(), rpos(), available()); } http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/110ee570/sshd-core/src/main/java/org/apache/sshd/common/util/buffer/ByteArrayBuffer.java ---------------------------------------------------------------------- diff --git a/sshd-core/src/main/java/org/apache/sshd/common/util/buffer/ByteArrayBuffer.java b/sshd-core/src/main/java/org/apache/sshd/common/util/buffer/ByteArrayBuffer.java index 8d29494..8e64647 100644 --- a/sshd-core/src/main/java/org/apache/sshd/common/util/buffer/ByteArrayBuffer.java +++ b/sshd-core/src/main/java/org/apache/sshd/common/util/buffer/ByteArrayBuffer.java @@ -168,11 +168,16 @@ public class ByteArrayBuffer extends Buffer { @Override public void getRawBytes(byte[] buf, int off, int len) { ensureAvailable(len); - System.arraycopy(data, rpos, buf, off, len); + copyRawBytes(0, buf, off, len); rpos += len; } @Override + protected void copyRawBytes(int offset, byte[] buf, int pos, int len) { + System.arraycopy(data, rpos + offset, buf, pos, len); + } + + @Override public void ensureCapacity(int capacity, Int2IntFunction growthFactor) { ValidateUtils.checkTrue(capacity >= 0, "Negative capacity requested: %d", capacity); http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/110ee570/sshd-core/src/test/java/org/apache/sshd/common/session/helpers/AbstractSessionTest.java ---------------------------------------------------------------------- diff --git a/sshd-core/src/test/java/org/apache/sshd/common/session/helpers/AbstractSessionTest.java b/sshd-core/src/test/java/org/apache/sshd/common/session/helpers/AbstractSessionTest.java index b0f4a89..ced4eb9 100644 --- a/sshd-core/src/test/java/org/apache/sshd/common/session/helpers/AbstractSessionTest.java +++ b/sshd-core/src/test/java/org/apache/sshd/common/session/helpers/AbstractSessionTest.java @@ -39,6 +39,7 @@ import org.apache.sshd.common.io.IoService; import org.apache.sshd.common.io.IoSession; import org.apache.sshd.common.io.IoWriteFuture; import org.apache.sshd.common.kex.KexProposalOption; +import org.apache.sshd.common.session.ReservedSessionMessagesHandler; import org.apache.sshd.common.session.Session; import org.apache.sshd.common.util.GenericUtils; import org.apache.sshd.common.util.buffer.Buffer; @@ -192,6 +193,82 @@ public class AbstractSessionTest extends BaseTestSupport { assertEquals("Close listener not called", 1, closeCount.get()); } + @Test // see SSHD-699 + public void testMalformedIgnoreMessageBadLength() throws Exception { + session.setReservedSessionMessagesHandler(new ReservedSessionMessagesHandler() { + @Override + public void handleIgnoreMessage(Session session, Buffer buffer) throws Exception { + fail("Unexpected invocation: available=" + buffer.available()); + } + }); + + Buffer buffer = new ByteArrayBuffer(Long.SIZE); + for (int index = 0; index < Integer.BYTES; index++) { + buffer.putByte((byte) index); + session.handleIgnore(buffer); + } + } + + @Test // see SSHD-699 + public void testMalformedIgnoreMessageCorruptedData() throws Exception { + session.setReservedSessionMessagesHandler(new ReservedSessionMessagesHandler() { + @Override + public void handleIgnoreMessage(Session session, Buffer buffer) throws Exception { + fail("Unexpected invocation: available=" + buffer.available()); + } + }); + + Buffer buffer = new ByteArrayBuffer(Long.SIZE + Byte.MAX_VALUE); + buffer.putInt(Byte.MAX_VALUE + 1); // bad message length + for (int index = 0; index < Byte.MAX_VALUE; index++) { + buffer.putByte((byte) index); + session.handleIgnore(buffer); + } + } + + @Test + public void testMalformedDebugMessageContent() throws Exception { + session.setReservedSessionMessagesHandler(new ReservedSessionMessagesHandler() { + @Override + public void handleDebugMessage(Session session, Buffer buffer) throws Exception { + fail("Unexpected invocation: available=" + buffer.available()); + } + }); + + Buffer buffer = new ByteArrayBuffer(Long.SIZE + Byte.MAX_VALUE); + session.handleDebug(buffer); // no boolean field + + buffer.putBoolean(true); + session.handleDebug(buffer); // no message field + + buffer.putInt(Byte.MAX_VALUE + 1); // bad message field length + for (int index = 0; index < Byte.MAX_VALUE; index++) { + buffer.putByte((byte) index); + session.handleDebug(buffer); + } + } + + @Test + public void testMalformedDebugMessageLanguage() throws Exception { + session.setReservedSessionMessagesHandler(new ReservedSessionMessagesHandler() { + @Override + public void handleDebugMessage(Session session, Buffer buffer) throws Exception { + fail("Unexpected invocation: available=" + buffer.available()); + } + }); + + Buffer buffer = new ByteArrayBuffer(Long.SIZE); + buffer.putBoolean(true); + buffer.putString(getCurrentTestName()); + session.handleDebug(buffer); // no language tag + + buffer.putInt(Byte.SIZE + 1); // bad language tag length + for (int index = 0; index < Byte.SIZE; index++) { + buffer.putByte((byte) index); + session.handleDebug(buffer); + } + } + private static String readIdentification(MySession session, Buffer buf) { List<String> lines = session.doReadIdentification(buf); return GenericUtils.isEmpty(lines) ? null : lines.get(lines.size() - 1);
