Fwd: mpi_set_secure leads to heap corruption
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
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.
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 +-
@@ -256,7 +256,7 @@ mpi_set_secure( gcry_mpi_t a )
- 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