Hello, (Cc-ed to the bug report BTS) Thank you for forwarding the bug report.
Fixed both for master and LIBGCRYPT-1-7-BRANCH. master: https://git.gnupg.org/cgi-bin/gitweb.cgi?p=libgcrypt.git;a=commit;h=5feaf1cc8f22c1f8d19a34850d86fe190f1432e2 LIBGCRYPT-1-7-BRANCH: https://git.gnupg.org/cgi-bin/gitweb.cgi?p=libgcrypt.git;a=commit;h=a195d7346a8006f3b6fb77ccd6df8e91833d2b5a > 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 surprise. While there is the API, I don't know the real use case. So, I did search: https://codesearch.debian.net/search?q=mpi_set_flag.*GCRYMPI_FLAG_SECURE 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 corruption. ================================================== 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 <[email protected]> Signed-off-by: NIIBE Yutaka <[email protected]> (backport from master commit: 5feaf1cc8f22c1f8d19a34850d86fe190f1432e2) 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); return; } - 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); --
