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

[Andreas Schneider <asn@xxxxxxxxxxxxxx>]

On Wednesday 05 February 2014 20:14:07 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 |    6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/src/packet_crypt.c b/src/packet_crypt.c
> index 50b8189..479cd16 100644
> --- a/src/packet_crypt.c
> +++ b/src/packet_crypt.c
> @@ -59,7 +59,8 @@ 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){
> +  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 SSH_ERROR;
>    }
> @@ -92,7 +93,8 @@ unsigned char *packet_encrypt(ssh_session session, void
> *data, uint32_t 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;
>    }

I think the only thing we should add here is assert(len). These functions 
should never be called if len is 0.


	-- andreas


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