Re: [PATCH v2 2/2] packet_crypt: Make packet_{en,de}crypt fail consistently on len == 0

[Alan Dunn <amdunn@xxxxxxxxx>]

OK.  Fixed in attached (though if you want I can instead resend the whole set).

On Thu, Feb 6, 2014 at 10:32 AM, Andreas Schneider <asn@xxxxxxxxxxxxxx> wrote:
> On Thursday 06 February 2014 10:26:20 Alan Dunn wrote:
>> Doesn't assert only take action when NDEBUG is undefined?  That means
>> if this error occurs in a release version, where I believe cmake will
>> define NDEBUG, then without the check we might crash with the
>> potentially difficult to understand "Decrypt error" (which was
>> actually how this whole thing started for me).  Unless we want some
>> other version of assert.
>
> The thing is these function should never been called if len is 0. It is a bug
> if we do so.
>
> The assert should be enough that if someone has an error he can enable
> debugging and will find out why.
>
>
>         -- andreas
>
> --
> Andreas Schneider                   GPG-ID: CC014E3D
> www.cryptomilk.org                asn@xxxxxxxxxxxxxx
>

From 79a806caaf52de969f3325ed69fd6666509324da Mon Sep 17 00:00:00 2001
From: Alan Dunn <amdunn@xxxxxxxxx>
Date: Wed, 5 Feb 2014 17:26:57 -0600
Subject: [PATCH v3 2/2] packet_crypt: Make packet_{en,de}crypt fail
 consistently on len == 0
To: libssh@xxxxxxxxxx

Right now the behavior of packet_{en,de}crypt on len == 0 depends on
the behavior of malloc.  Instead, make these consistently fail based
on what I assume the desired behavior is due to the first error
message in each.

Signed-off-by: Alan Dunn <amdunn@xxxxxxxxx>
---
 src/packet_crypt.c |    6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/src/packet_crypt.c b/src/packet_crypt.c
index 50b8189..cb73e41 100644
--- a/src/packet_crypt.c
+++ b/src/packet_crypt.c
@@ -22,6 +22,7 @@
  */
 
 #include "config.h"
+#include <assert.h>
 #include <stdlib.h>
 #include <stdio.h>
 #include <string.h>
@@ -59,6 +60,9 @@ uint32_t packet_decrypt_len(ssh_session session, char *crypted){
 int packet_decrypt(ssh_session session, void *data,uint32_t len) {
   struct ssh_cipher_struct *crypto = session->current_crypto->in_cipher;
   char *out = NULL;
+
+  assert(len);
+
   if(len % session->current_crypto->in_cipher->blocksize != 0){
     ssh_set_error(session, SSH_FATAL, "Cryptographic functions must be set on at least one blocksize (received %d)",len);
     return SSH_ERROR;
@@ -89,6 +93,8 @@ unsigned char *packet_encrypt(ssh_session session, void *data, uint32_t len) {
   unsigned int finallen;
   uint32_t seq;
 
+  assert(len);
+
   if (!session->current_crypto) {
     return NULL; /* nothing to do here */
   }
-- 
1.7.9.5