packet: Do not decrypt zero length rest of buffer

If we receive a packet of length exactly blocksize, then
packet_decrypt gets called on a buffer of size 0.  The check at the
beginning of packet_decrypt indicates that the function should be
called on buffers of at least one blocksize, though the check allows
through zero length.  As is packet_decrypt can return -1 when len is 0
because malloc can return NULL in this case: according to the ISO C
standard, malloc is free to return NULL or a pointer that can be freed
when size == 0, and uclibc by default will return NULL here (in
"non-glibc-compatible" mode).  The net result is that when using
uclibc connections with libssh can anomalously fail.

Alternatively, packet_decrypt (and probably packet_encrypt for
consistency) could be made to always succeed on len == 0 without
depending on the behavior of malloc.

Thanks to Josh Berlin for bringing conneciton failures with uclibc to
my attention.

Signed-off-by: Alan Dunn <amdunn@gmail.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>

2 files changed, 25 insertions, 13 deletions

diff --git a/src/packet.c b/src/packet.c
index 75f6b58f..ffd08d06 100644
--- a/src/packet.c
+++ b/src/packet.c
@@ -152,7 +152,7 @@ int ssh_packet_socket_callback(const void *data, size_t receivedlen, void *user)
     const uint8_t *packet;
     int to_be_read;
     int rc;
-    uint32_t len, compsize, payloadsize;
+    uint32_t len, compsize, payloadsize, buffer_len;
     uint8_t padding;
     size_t processed = 0; /* number of byte processed from the callback */
 
@@ -251,12 +251,17 @@ int ssh_packet_socket_callback(const void *data, size_t receivedlen, void *user)
                  * Decrypt the rest of the packet (blocksize bytes already
                  * have been decrypted)
                  */
-                rc = packet_decrypt(session,
-                                    ((uint8_t*)buffer_get_rest(session->in_buffer) + blocksize),
-                                    buffer_get_rest_len(session->in_buffer) - blocksize);
-                if (rc < 0) {
-                    ssh_set_error(session, SSH_FATAL, "Decrypt error");
-                    goto error;
+
+                /* The following check avoids decrypting zero bytes */
+                buffer_len = buffer_get_rest_len(session->in_buffer);
+                if (buffer_len != blocksize) {
+                    rc = packet_decrypt(session,
+                                        ((uint8_t*)buffer_get_rest(session->in_buffer) + blocksize),
+                                        buffer_len - blocksize);
+                    if (rc < 0) {
+                        ssh_set_error(session, SSH_FATAL, "Decrypt error");
+                        goto error;
+                    }
                 }
 
                 /* copy the last part from the incoming buffer */
diff --git a/src/packet1.c b/src/packet1.c
index 7ac8318d..127bb776 100644
--- a/src/packet1.c
+++ b/src/packet1.c
@@ -106,7 +106,7 @@ int ssh_packet_socket_callback1(const void *data, size_t receivedlen, void *user
   size_t processed=0;
   uint32_t padding;
   uint32_t crc;
-  uint32_t len;
+  uint32_t len, buffer_len;
   ssh_session session=(ssh_session)user;
 
   switch (session->packet_state){
@@ -168,11 +168,16 @@ int ssh_packet_socket_callback1(const void *data, size_t receivedlen, void *user
          * We decrypt everything, missing the lenght part (which was
          * previously read, unencrypted, and is not part of the buffer
          */
-        if (packet_decrypt(session,
-              ssh_buffer_get_begin(session->in_buffer),
-              ssh_buffer_get_len(session->in_buffer)) < 0) {
-          ssh_set_error(session, SSH_FATAL, "Packet decrypt error");
-          goto error;
+        buffer_len = ssh_buffer_get_len(session->in_buffer);
+        if (buffer_len > 0) {
+          int rc;
+          rc = packet_decrypt(session,
+                 ssh_buffer_get_begin(session->in_buffer),
+                 buffer_len);
+          if (rc < 0) {
+            ssh_set_error(session, SSH_FATAL, "Packet decrypt error");
+            goto error;
+          }
         }
       }
 #ifdef DEBUG_CRYPTO
@@ -300,6 +305,8 @@ int packet_send1(ssh_session session) {
       ssh_buffer_get_len(session->out_buffer));
 #endif
 
+  /* session->out_buffer should have more than sizeof(uint32_t) bytes
+     in it as required for packet_encrypt */
   packet_encrypt(session, (unsigned char *)ssh_buffer_get_begin(session->out_buffer) + sizeof(uint32_t),
       ssh_buffer_get_len(session->out_buffer) - sizeof(uint32_t));