Fwd: mpi_set_secure leads to heap corruption

[NIIBE Yutaka]

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 <mdw at distorted.org.uk>
Signed-off-by: NIIBE Yutaka <gniibe at fsij.org>

(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);

--