[Pkg-gnupg-maint] Bug#773521: Bug#773521: incorrect memset

[NIIBE Yutaka]

Hello,

Thanks for your reviewing and reporting.

On 12/19/2014 10:04 PM, Joshua Rogers wrote:
> Package: gnupg2
> Version: 2.1.1
> Severity: normal
[...]
> on line 253 of ecdh.c, memset is called with a 0 fill value, which
> will do nothing. what's the point?

Well, I guess that the intention of the code is:

  (1) Avoid calling allocator twice, but allocating larger space at
      the beginning and reuse it for multiple purposes in its
      computation.

  (2) Since the memory is used for secret data, it's better to fill
      zero (or something unimportant) after its use.

In my opinion, this memset is not needed since it's allocated by
xtrymalloc_secure and it's soon freed at line 278 (or 273).

Nevertheless, I found the following fix in the repo.

commit 56e688823345bbcfef220b13eb418854f8798b16
Author: Werner Koch <wk at gnupg.org>
Date:   Mon Jan 5 15:03:12 2015 +0100

    gpg: Clear a possible rest of the KDF secret buffer.

    * g10/ecdh.c (pk_ecdh_encrypt_with_shared_point): Fix order of args.
    --

    That bug has been here since the beginning.  The entire function needs
    a review or be be moved to Libgcrypt.

    Signed-off-by: Werner Koch <wk at gnupg.org>

diff --git a/g10/ecdh.c b/g10/ecdh.c
index 0b06239..07f3983 100644
--- a/g10/ecdh.c
+++ b/g10/ecdh.c
@@ -250,7 +250,7 @@ pk_ecdh_encrypt_with_shared_point (int is_encrypt, gcry_mpi_t shared_mpi,
     assert( secret_x_size <= gcry_md_get_algo_dlen (kdf_hash_algo) );

     /* We could have allocated more, so clean the tail before returning.  */
-    memset( secret_x+secret_x_size, old_size-secret_x_size, 0 );
+    memset (secret_x+secret_x_size, 0, old_size - secret_x_size);
     if (DBG_CIPHER)
       log_printhex ("ecdh KEK is:", secret_x, secret_x_size );
   }
--