[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 <lyor.goldst...@gmail.com>
Authored: Fri Sep 16 18:16:26 2016 +0300
Committer: Lyor Goldstein <lyor.goldst...@gmail.com>
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);

Reply via email to