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

[Andreas Schneider <asn@xxxxxxxxxxxxxx>]

On Thursday 06 February 2014 08:12:47 Alan Dunn wrote:
> 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 |   13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/src/packet_crypt.c b/src/packet_crypt.c
> index 50b8189..a12e3d9 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,7 +60,11 @@ 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;
> -  if(len % session->current_crypto->in_cipher->blocksize != 0){
> +
> +  assert(len);
> +
> +  if(len == 0 ||

You can remove this check, assert() calls abort() in case it is 0 and that's 
what we really want :)

We want to crash if len is 0. :)

> +     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,10 +94,14 @@ 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 */
>    }
> -  if(len % session->current_crypto->in_cipher->blocksize != 0){
> +
> +  if(len == 0 ||
> +     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 NULL;
>    }

-- 
Andreas Schneider                   GPG-ID: CC014E3D
www.cryptomilk.org                asn@xxxxxxxxxxxxxx