From dedcbd106f8e52d5586b0205bc7677e4c9868f9c Mon Sep 17 00:00:00 2001
From: Will Cosgrove <will@panic.com>
Date: Fri, 30 Aug 2019 09:57:38 -0700
Subject: [PATCH] packet.c: improve message parsing (#402)

* packet.c: improve parsing of packets

file: packet.c

notes:
Use _libssh2_get_string API in SSH_MSG_DEBUG/SSH_MSG_DISCONNECT. Additional uint32 bounds check in SSH_MSG_GLOBAL_REQUEST.
---
 src/packet.c | 68 ++++++++++++++++++++++------------------------------
 1 file changed, 29 insertions(+), 39 deletions(-)

Index: libssh2-1.4.3/src/packet.c
===================================================================
--- libssh2-1.4.3.orig/src/packet.c
+++ libssh2-1.4.3/src/packet.c
@@ -416,8 +416,8 @@ _libssh2_packet_add(LIBSSH2_SESSION * se
                     size_t datalen, int macstate)
 {
     int rc = 0;
-    char *message=NULL;
-    char *language=NULL;
+    unsigned char *message = NULL;
+    unsigned char *language = NULL;
     size_t message_len=0;
     size_t language_len=0;
     LIBSSH2_CHANNEL *channelp = NULL;
@@ -469,32 +469,23 @@ _libssh2_packet_add(LIBSSH2_SESSION * se
 
         case SSH_MSG_DISCONNECT:
             if(datalen >= 5) {
-                size_t reason = _libssh2_ntohu32(data + 1);
+                uint32_t reason = 0;
+                struct string_buf buf;
+                buf.data = (unsigned char *)data;
+                buf.dataptr = buf.data;
+                buf.len = datalen;
+                buf.dataptr++; /* advance past type */
+
+                _libssh2_get_u32(&buf, &reason);
+                _libssh2_get_string(&buf, &message, &message_len);
+                _libssh2_get_string(&buf, &language, &language_len);
 
-                if(datalen >= 9) {
-                    message_len = _libssh2_ntohu32(data + 5);
-
-                    if(message_len < datalen-13) {
-                        /* 9 = packet_type(1) + reason(4) + message_len(4) */
-                        message = (char *) data + 9;
-
-                        language_len = _libssh2_ntohu32(data + 9 + message_len);
-                        language = (char *) data + 9 + message_len + 4;
-
-                        if(language_len > (datalen-13-message_len)) {
-                            /* bad input, clear info */
-                            language = message = NULL;
-                            language_len = message_len = 0;
-                        }
-                    }
-                    else
-                        /* bad size, clear it */
-                        message_len=0;
-                }
                 if (session->ssh_msg_disconnect) {
-                    LIBSSH2_DISCONNECT(session, reason, message,
-                                       message_len, language, language_len);
+			LIBSSH2_DISCONNECT(session, reason, (const char *)message,
+					   message_len, (const char *)language,
+					   language_len);
                 }
+
                 _libssh2_debug(session, LIBSSH2_TRACE_TRANS,
                                "Disconnect(%d): %s(%s)", reason,
                                message, language);
@@ -534,23 +525,24 @@ _libssh2_packet_add(LIBSSH2_SESSION * se
                 int always_display= data[1];
 
                 if(datalen >= 6) {
-                    message_len = _libssh2_ntohu32(data + 2);
+			struct string_buf buf;
+			buf.data = (unsigned char *)data;
+			buf.dataptr = buf.data;
+			buf.len = datalen;
+			buf.dataptr += 2; /* advance past type & always display */
 
-                    if(message_len <= (datalen - 10)) {
-                        /* 6 = packet_type(1) + display(1) + message_len(4) */
-                        message = (char *) data + 6;
-                        language_len = _libssh2_ntohu32(data + 6 + message_len);
-
-                        if(language_len <= (datalen - 10 - message_len))
-                            language = (char *) data + 10 + message_len;
-                    }
+			_libssh2_get_string(&buf, &message, &message_len);
+			_libssh2_get_string(&buf, &language, &language_len);
                 }
 
                 if (session->ssh_msg_debug) {
-                    LIBSSH2_DEBUG(session, always_display, message,
-                                  message_len, language, language_len);
+			LIBSSH2_DEBUG(session, always_display,
+				      (const char *)message,
+				      message_len, (const char *)language,
+				      language_len);
                 }
             }
+
             /*
              * _libssh2_debug will actually truncate this for us so
              * that it's not an inordinate about of data
@@ -573,7 +565,7 @@ _libssh2_packet_add(LIBSSH2_SESSION * se
                 uint32_t len =0;
                 unsigned char want_reply=0;
                 len = _libssh2_ntohu32(data + 1);
-                if(datalen >= (6 + len)) {
+                if((len <= (UINT_MAX - 6)) && (datalen >= (6 + len))) {
                     want_reply = data[5 + len];
                     _libssh2_debug(session,
                                    LIBSSH2_TRACE_CONN,
Index: libssh2-1.4.3/src/misc.c
===================================================================
--- libssh2-1.4.3.orig/src/misc.c
+++ libssh2-1.4.3/src/misc.c
@@ -610,3 +610,44 @@ int __cdecl _libssh2_gettimeofday(struct
 
 
 #endif
+
+/* Given the current location in buf, _libssh2_check_length ensures
+   callers can read the next len number of bytes out of the buffer
+   before reading the buffer content */
+
+int _libssh2_check_length(struct string_buf *buf, size_t len)
+{
+    unsigned char *endp = &buf->data[buf->len];
+    size_t left = endp - buf->dataptr;
+    return ((len <= left) && (left <= buf->len));
+}
+
+int _libssh2_get_u32(struct string_buf *buf, uint32_t *out)
+{
+    if(!_libssh2_check_length(buf, 4)) {
+        return -1;
+    }
+
+    *out = _libssh2_ntohu32(buf->dataptr);
+    buf->dataptr += 4;
+    return 0;
+}
+
+int _libssh2_get_string(struct string_buf *buf, unsigned char **outbuf,
+                        size_t *outlen)
+{
+    uint32_t data_len;
+    if(_libssh2_get_u32(buf, &data_len) != 0) {
+        return -1;
+    }
+    if(!_libssh2_check_length(buf, data_len)) {
+        return -1;
+    }
+    *outbuf = buf->dataptr;
+    buf->dataptr += data_len;
+
+    if(outlen)
+        *outlen = (size_t)data_len;
+
+    return 0;
+}
Index: libssh2-1.4.3/src/misc.h
===================================================================
--- libssh2-1.4.3.orig/src/misc.h
+++ libssh2-1.4.3/src/misc.h
@@ -49,6 +49,12 @@ struct list_node {
     struct list_head *head;
 };
 
+struct string_buf {
+    unsigned char *data;
+    unsigned char *dataptr;
+    size_t len;
+};
+
 int _libssh2_error(LIBSSH2_SESSION* session, int errcode, const char* errmsg);
 
 void _libssh2_list_init(struct list_head *head);
@@ -78,6 +84,11 @@ void _libssh2_htonu32(unsigned char *buf
 void _libssh2_store_u32(unsigned char **buf, uint32_t value);
 void _libssh2_store_str(unsigned char **buf, const char *str, size_t len);
 
+int _libssh2_get_u32(struct string_buf *buf, uint32_t *out);
+int _libssh2_get_string(struct string_buf *buf, unsigned char **outbuf,
+                        size_t *outlen);
+int _libssh2_check_length(struct string_buf *buf, size_t requested_len);
+
 #if defined(LIBSSH2_WIN32) && !defined(__MINGW32__) && !defined(__CYGWIN__)
 /* provide a private one */
 #undef HAVE_GETTIMEOFDAY
