mpi_swap_cond: different sizes error on eddsa key generation

NIIBE Yutaka gniibe at fsij.org
Tue Dec 20 05:53:27 CET 2016


Kostis Andrikopoulos <el11151 at mail.ntua.gr> wrote:
> Thanks for the thorough explanation of the code related to the bug. I
> think i have isolated the buggy code enough to be able to reach some
> conclusions.

Good.  Thank you for the reproducible case.

I think that the problem is here:

> static int otrl_mem_is_secure(const void *p)
> {
>     return 1;
> }

(and sharing same code for normal memory allocation and
secure memmory allocation.)

It breaks the assumption of normally allocated memory keeps this
characteristic.  From the view point of libgcrypt, suddenly, normally
allocated memory becomes secure one.

If such a thing could occur, we need to change libgcrypt so that
_gcry_mpi_assign_limb_space should be always called with memory of
no-smaller size than the original.

Here is a possible change.  Currently, I don't know if it's the
right thing.  Let us discuss.

===========================================
diff --git a/mpi/mpi-mul.c b/mpi/mpi-mul.c
index 4f4d709..313dc25 100644
--- a/mpi/mpi-mul.c
+++ b/mpi/mpi-mul.c
@@ -110,6 +110,7 @@ void
 _gcry_mpi_mul (gcry_mpi_t w, gcry_mpi_t u, gcry_mpi_t v)
 {
     mpi_size_t usize, vsize, wsize;
+    mpi_size_t w_alloced;
     mpi_ptr_t up, vp, wp;
     mpi_limb_t cy;
     int usign, vsign, usecure, vsecure, sign_product;
@@ -142,11 +143,12 @@ _gcry_mpi_mul (gcry_mpi_t w, gcry_mpi_t u, gcry_mpi_t v)
 
     /* Ensure W has space enough to store the result.  */
     wsize = usize + vsize;
+    w_alloced = (wsize > w->alloced)? wsize : w->alloced;
     if ( !mpi_is_secure (w) && (mpi_is_secure (u) || mpi_is_secure (v)) ) {
         /* w is not allocated in secure space but u or v is.  To make sure
          * that no temporray results are stored in w, we temporary use
          * a newly allocated limb space for w */
-        wp = mpi_alloc_limb_space( wsize, 1 );
+        wp = mpi_alloc_limb_space( w_alloced, 1 );
         assign_wp = 2; /* mark it as 2 so that we can later copy it back to
                         * mormal memory */
     }
@@ -190,12 +192,12 @@ _gcry_mpi_mul (gcry_mpi_t w, gcry_mpi_t u, gcry_mpi_t v)
     if( assign_wp ) {
         if (assign_wp == 2) {
             /* copy the temp wp from secure memory back to normal memory */
-	    mpi_ptr_t tmp_wp = mpi_alloc_limb_space (wsize, 0);
-	    MPN_COPY (tmp_wp, wp, wsize);
+            mpi_ptr_t tmp_wp = mpi_alloc_limb_space (w_alloced, 0);
+            MPN_COPY (tmp_wp, wp, wsize);
             _gcry_mpi_free_limb_space (wp, 0);
             wp = tmp_wp;
         }
-	_gcry_mpi_assign_limb_space( w, wp, wsize );
+        _gcry_mpi_assign_limb_space( w, wp, w_alloced );
     }
     w->nlimbs = wsize;
     w->sign = sign_product;
-- 



More information about the Gcrypt-devel mailing list