cipher: Fix possible NULL deref in call to prime generator.
authorWerner Koch <wk@gnupg.org>
Thu, 21 Aug 2014 09:39:17 +0000 (11:39 +0200)
committerWerner Koch <wk@gnupg.org>
Thu, 21 Aug 2014 10:22:37 +0000 (12:22 +0200)
* cipher/primegen.c (_gcry_generate_elg_prime): Change to return an
error code.
* cipher/dsa.c (generate): Take care of new return code.
* cipher/elgamal.c (generate): Change to return an error code.  Take
care of _gcry_generate_elg_prime return code.
(generate_using_x): Take care of _gcry_generate_elg_prime return code.
(elg_generate): Propagate return code from generate.
--

GnuPG-bug-id: 1699, 1700
Reported-by: S.K. Gupta
Note that the NULL deref may have only happened on malloc failure.

cipher/dsa.c
cipher/elgamal.c
cipher/primegen.c
src/g10lib.h

index 1707d8c..09cd969 100644 (file)
@@ -196,6 +196,7 @@ static gpg_err_code_t
 generate (DSA_secret_key *sk, unsigned int nbits, unsigned int qbits,
           int transient_key, dsa_domain_t *domain, gcry_mpi_t **ret_factors )
 {
 generate (DSA_secret_key *sk, unsigned int nbits, unsigned int qbits,
           int transient_key, dsa_domain_t *domain, gcry_mpi_t **ret_factors )
 {
+  gpg_err_code_t rc;
   gcry_mpi_t p;    /* the prime */
   gcry_mpi_t q;    /* the 160 bit prime factor */
   gcry_mpi_t g;    /* the generator */
   gcry_mpi_t p;    /* the prime */
   gcry_mpi_t q;    /* the 160 bit prime factor */
   gcry_mpi_t g;    /* the generator */
@@ -247,7 +248,10 @@ generate (DSA_secret_key *sk, unsigned int nbits, unsigned int qbits,
   else
     {
       /* Generate new domain parameters.  */
   else
     {
       /* Generate new domain parameters.  */
-      p = _gcry_generate_elg_prime (1, nbits, qbits, NULL, ret_factors);
+      rc = _gcry_generate_elg_prime (1, nbits, qbits, NULL, &p, ret_factors);
+      if (rc)
+        return rc;
+
       /* Get q out of factors.  */
       q = mpi_copy ((*ret_factors)[0]);
       gcry_assert (mpi_get_nbits (q) == qbits);
       /* Get q out of factors.  */
       q = mpi_copy ((*ret_factors)[0]);
       gcry_assert (mpi_get_nbits (q) == qbits);
index a71a9bc..cb3ca43 100644 (file)
@@ -61,7 +61,8 @@ static const char *elg_names[] =
 
 static int test_keys (ELG_secret_key *sk, unsigned int nbits, int nodie);
 static gcry_mpi_t gen_k (gcry_mpi_t p, int small_k);
 
 static int test_keys (ELG_secret_key *sk, unsigned int nbits, int nodie);
 static gcry_mpi_t gen_k (gcry_mpi_t p, int small_k);
-static void generate (ELG_secret_key *sk, unsigned nbits, gcry_mpi_t **factors);
+static gcry_err_code_t generate (ELG_secret_key *sk, unsigned nbits,
+                                 gcry_mpi_t **factors);
 static int  check_secret_key (ELG_secret_key *sk);
 static void do_encrypt (gcry_mpi_t a, gcry_mpi_t b, gcry_mpi_t input,
                         ELG_public_key *pkey);
 static int  check_secret_key (ELG_secret_key *sk);
 static void do_encrypt (gcry_mpi_t a, gcry_mpi_t b, gcry_mpi_t input,
                         ELG_public_key *pkey);
@@ -268,9 +269,10 @@ gen_k( gcry_mpi_t p, int small_k )
  * Returns: 2 structures filled with all needed values
  *         and an array with n-1 factors of (p-1)
  */
  * Returns: 2 structures filled with all needed values
  *         and an array with n-1 factors of (p-1)
  */
-static void
+static gcry_err_code_t
 generate ( ELG_secret_key *sk, unsigned int nbits, gcry_mpi_t **ret_factors )
 {
 generate ( ELG_secret_key *sk, unsigned int nbits, gcry_mpi_t **ret_factors )
 {
+  gcry_err_code_t rc;
   gcry_mpi_t p;    /* the prime */
   gcry_mpi_t p_min1;
   gcry_mpi_t g;
   gcry_mpi_t p;    /* the prime */
   gcry_mpi_t p_min1;
   gcry_mpi_t g;
@@ -285,7 +287,13 @@ generate ( ELG_secret_key *sk, unsigned int nbits, gcry_mpi_t **ret_factors )
   if( qbits & 1 ) /* better have a even one */
     qbits++;
   g = mpi_alloc(1);
   if( qbits & 1 ) /* better have a even one */
     qbits++;
   g = mpi_alloc(1);
-  p = _gcry_generate_elg_prime( 0, nbits, qbits, g, ret_factors );
+  rc = _gcry_generate_elg_prime (0, nbits, qbits, g, &p, ret_factors);
+  if (rc)
+    {
+      mpi_free (p_min1);
+      mpi_free (g);
+      return rc;
+    }
   mpi_sub_ui(p_min1, p, 1);
 
 
   mpi_sub_ui(p_min1, p, 1);
 
 
@@ -359,6 +367,8 @@ generate ( ELG_secret_key *sk, unsigned int nbits, gcry_mpi_t **ret_factors )
 
   /* Now we can test our keys (this should never fail!) */
   test_keys ( sk, nbits - 64, 0 );
 
   /* Now we can test our keys (this should never fail!) */
   test_keys ( sk, nbits - 64, 0 );
+
+  return 0;
 }
 
 
 }
 
 
@@ -373,6 +383,7 @@ static gcry_err_code_t
 generate_using_x (ELG_secret_key *sk, unsigned int nbits, gcry_mpi_t x,
                   gcry_mpi_t **ret_factors )
 {
 generate_using_x (ELG_secret_key *sk, unsigned int nbits, gcry_mpi_t x,
                   gcry_mpi_t **ret_factors )
 {
+  gcry_err_code_t rc;
   gcry_mpi_t p;      /* The prime.  */
   gcry_mpi_t p_min1; /* The prime minus 1.  */
   gcry_mpi_t g;      /* The generator.  */
   gcry_mpi_t p;      /* The prime.  */
   gcry_mpi_t p_min1; /* The prime minus 1.  */
   gcry_mpi_t g;      /* The generator.  */
@@ -395,7 +406,13 @@ generate_using_x (ELG_secret_key *sk, unsigned int nbits, gcry_mpi_t x,
   if ( (qbits & 1) ) /* Better have an even one.  */
     qbits++;
   g = mpi_alloc (1);
   if ( (qbits & 1) ) /* Better have an even one.  */
     qbits++;
   g = mpi_alloc (1);
-  p = _gcry_generate_elg_prime ( 0, nbits, qbits, g, ret_factors );
+  rc = _gcry_generate_elg_prime (0, nbits, qbits, g, &p, ret_factors );
+  if (rc)
+    {
+      mpi_free (p_min1);
+      mpi_free (g);
+      return rc;
+    }
   mpi_sub_ui (p_min1, p, 1);
 
   if (DBG_CIPHER)
   mpi_sub_ui (p_min1, p, 1);
 
   if (DBG_CIPHER)
@@ -662,8 +679,7 @@ elg_generate (const gcry_sexp_t genparms, gcry_sexp_t *r_skey)
     }
   else
     {
     }
   else
     {
-      generate (&sk, nbits, &factors);
-      rc = 0;
+      rc = generate (&sk, nbits, &factors);
     }
   if (rc)
     goto leave;
     }
   if (rc)
     goto leave;
index 9f6ec70..14a5ccf 100644 (file)
@@ -726,19 +726,22 @@ prime_generate_internal (int need_q_factor,
 
 
 /* Generate a prime used for discrete logarithm algorithms; i.e. this
 
 
 /* Generate a prime used for discrete logarithm algorithms; i.e. this
-   prime will be public and no strong random is required.  */
-gcry_mpi_t
+   prime will be public and no strong random is required.  On success
+   R_PRIME receives a new MPI with the prime.  On error R_PRIME is set
+   to NULL and an error code is returned.  If RET_FACTORS is not NULL
+   it is set to an allocated array of factors on success or to NULL on
+   error.  */
+gcry_err_code_t
 _gcry_generate_elg_prime (int mode, unsigned pbits, unsigned qbits,
 _gcry_generate_elg_prime (int mode, unsigned pbits, unsigned qbits,
-                         gcry_mpi_t g, gcry_mpi_t **ret_factors)
+                         gcry_mpi_t g,
+                          gcry_mpi_t *r_prime, gcry_mpi_t **ret_factors)
 {
 {
-  gcry_mpi_t prime = NULL;
-
-  if (prime_generate_internal ((mode == 1), &prime, pbits, qbits, g,
-                               ret_factors, GCRY_WEAK_RANDOM, 0, 0,
-                               NULL, NULL))
-    prime = NULL; /* (Should be NULL in the error case anyway.)  */
-
-  return prime;
+  *r_prime = NULL;
+  if (ret_factors)
+    *ret_factors = NULL;
+  return prime_generate_internal ((mode == 1), r_prime, pbits, qbits, g,
+                                  ret_factors, GCRY_WEAK_RANDOM, 0, 0,
+                                  NULL, NULL);
 }
 
 
 }
 
 
index 43dc011..238871d 100644 (file)
@@ -236,9 +236,12 @@ gcry_mpi_t _gcry_generate_public_prime (unsigned int nbits,
                                  gcry_random_level_t random_level,
                                  int (*extra_check)(void*, gcry_mpi_t),
                                  void *extra_check_arg);
                                  gcry_random_level_t random_level,
                                  int (*extra_check)(void*, gcry_mpi_t),
                                  void *extra_check_arg);
-gcry_mpi_t _gcry_generate_elg_prime (int mode,
-                                     unsigned int pbits, unsigned int qbits,
-                                     gcry_mpi_t g, gcry_mpi_t **factors);
+gcry_err_code_t _gcry_generate_elg_prime (int mode,
+                                          unsigned int pbits,
+                                          unsigned int qbits,
+                                          gcry_mpi_t g,
+                                          gcry_mpi_t *r_prime,
+                                          gcry_mpi_t **factors);
 gcry_mpi_t _gcry_derive_x931_prime (const gcry_mpi_t xp,
                                     const gcry_mpi_t xp1, const gcry_mpi_t xp2,
                                     const gcry_mpi_t e,
 gcry_mpi_t _gcry_derive_x931_prime (const gcry_mpi_t xp,
                                     const gcry_mpi_t xp1, const gcry_mpi_t xp2,
                                     const gcry_mpi_t e,