Fwd: mpi_set_secure leads to heap corruption

NIIBE Yutaka gniibe at fsij.org
Tue Jul 4 03:05:11 CEST 2017

(Cc-ed to the bug report BTS)

Thank you for forwarding the bug report.

Fixed both for master and LIBGCRYPT-1-7-BRANCH.



> The patch fixes the problem.  I've not thought deeply about the
> performance effects: maybe it'd be better to allocate the same total
> limb buffer rather than just the active size, but this patch is simple
> and obviously right.

Yes.  While the patch is right, I followed the suggestion for less

While there is the API, I don't know the real use case.  So, I did


and seccure-0.5_1 has use cases.  Since all use cases are gcry_mpi_scan
then gcry_mpi_set_flag, I think that those cases are safe for heap

Commit: a195d7346a8006f3b6fb77ccd6df8e91833d2b5a

mpi: Fix mpi_set_secure.

* mpi/mpiutil.c (mpi_set_secure): Allocate by ->alloced.


The code was simply wrong.  The question is if (1) it allocates
(possibly) more or (2) modifi ->alloced.  The choice is (1).

Because we have routines of mpi_set_cond and mpi_swap_cond which
assume no change for the allocated length of limbs, no surprise is
better.  See _gcry_mpi_ec_mul_point for concrete example for those
routines.  That's for constant-time computation.

Debian-bug-id: 866964
Suggested-by: Mark Wooding <mdw at distorted.org.uk>
Signed-off-by: NIIBE Yutaka <gniibe at fsij.org>

(backport from master commit:

1 file changed, 1 insertion(+), 1 deletion(-)
mpi/mpiutil.c | 2 +-

modified   mpi/mpiutil.c
@@ -256,7 +256,7 @@ mpi_set_secure( gcry_mpi_t a )
       gcry_assert (!ap);
-  bp = mpi_alloc_limb_space (a->nlimbs, 1);
+  bp = mpi_alloc_limb_space (a->alloced, 1);
   MPN_COPY( bp, ap, a->nlimbs );
   a->d = bp;
   _gcry_mpi_free_limb_space (ap, a->alloced);


More information about the Gcrypt-devel mailing list