cipher: fix memory leaks
NIIBE Yutaka
gniibe at fsij.org
Tue Aug 6 10:35:48 CEST 2013
On 2013-08-06 at 09:48 +0200, Werner Koch wrote:
> Please push your fixes; you may want to merge them first, though.
Done.
For ECC ("pabgnqd", with no Q, but D), I try following patch. It
works, but doesn't look so good.
diff --git a/cipher/pubkey.c b/cipher/pubkey.c
index e867169..4bf74d7 100644
--- a/cipher/pubkey.c
+++ b/cipher/pubkey.c
@@ -304,13 +304,16 @@ _gcry_pk_unregister (gcry_module_t module)
}
static void
-release_mpi_array (gcry_mpi_t *array)
+release_mpi_array (gcry_mpi_t *array, size_t n)
{
- for (; *array; array++)
- {
- mpi_free(*array);
- *array = NULL;
- }
+ int i;
+
+ for (i = 0; i < n; i++)
+ if (array[i])
+ {
+ mpi_free (array[i]);
+ array[i] = NULL;
+ }
}
/****************
@@ -1854,11 +1857,7 @@ sexp_elements_extract (gcry_sexp_t key_sexp, const char *element_names,
if (err)
- {
- for (i = 0; i < idx; i++)
- if (elements[i])
- mpi_free (elements[i]);
- }
+ release_mpi_array (elements, idx);
return err;
}
@@ -1875,6 +1874,7 @@ sexp_elements_extract_ecc (gcry_sexp_t key_sexp, const char *element_names,
int idx;
const char *name;
gcry_sexp_t list;
+ size_t element_size;
/* Clear the array for easier error cleanup. */
for (name = element_names, idx = 0; *name; name++, idx++)
@@ -1883,7 +1883,7 @@ sexp_elements_extract_ecc (gcry_sexp_t key_sexp, const char *element_names,
(params only) or 6 (full public key). */
if (idx == 5)
elements[5] = NULL; /* Extra clear for the params only case. */
-
+ element_size = idx;
/* Init the array with the available curve parameters. */
for (name = element_names, idx = 0; *name && !err; name++, idx++)
@@ -1960,11 +1960,7 @@ sexp_elements_extract_ecc (gcry_sexp_t key_sexp, const char *element_names,
leave:
if (err)
- {
- for (name = element_names, idx = 0; *name; name++, idx++)
- if (elements[idx])
- mpi_free (elements[idx]);
- }
+ release_mpi_array (elements, element_size);
return err;
}
@@ -2009,8 +2005,8 @@ sexp_elements_extract_ecc (gcry_sexp_t key_sexp, const char *element_names,
*/
static gcry_err_code_t
sexp_to_key (gcry_sexp_t sexp, int want_private, int use,
- const char *override_elems,
- gcry_mpi_t **retarray, gcry_module_t *retalgo, int *r_is_ecc)
+ const char *override_elems, gcry_mpi_t **retarray,
+ size_t *retsize, gcry_module_t *retalgo, int *r_is_ecc)
{
gcry_err_code_t err = 0;
gcry_sexp_t list, l2;
@@ -2087,7 +2083,8 @@ sexp_to_key (gcry_sexp_t sexp, int want_private, int use,
elems = pubkey->elements_skey;
else
elems = pubkey->elements_pkey;
- array = gcry_calloc (strlen (elems) + 1, sizeof (*array));
+ *retsize = strlen (elems) + 1; /* We need +1 for ECC. */
+ array = gcry_calloc (*retsize, sizeof (*array));
if (!array)
err = gpg_err_code_from_syserror ();
if (!err)
@@ -2122,7 +2119,7 @@ sexp_to_key (gcry_sexp_t sexp, int want_private, int use,
static gcry_err_code_t
-sexp_to_sig (gcry_sexp_t sexp, gcry_mpi_t **retarray,
+sexp_to_sig (gcry_sexp_t sexp, gcry_mpi_t **retarray, size_t *retsize,
gcry_module_t *retalgo)
{
gcry_err_code_t err = 0;
@@ -2182,7 +2179,8 @@ sexp_to_sig (gcry_sexp_t sexp, gcry_mpi_t **retarray,
pubkey = (gcry_pk_spec_t *) module->spec;
elems = pubkey->elements_sig;
- array = gcry_calloc (strlen (elems) + 1 , sizeof *array );
+ *retsize = strlen (elems);
+ array = gcry_calloc (*retsize, sizeof *array );
if (!array)
err = gpg_err_code_from_syserror ();
@@ -2279,8 +2277,9 @@ get_hash_algo (const char *s, size_t n)
* case raw encoding is used.
*/
static gcry_err_code_t
-sexp_to_enc (gcry_sexp_t sexp, gcry_mpi_t **retarray, gcry_module_t *retalgo,
- int *ret_modern, int *flags, struct pk_encoding_ctx *ctx)
+sexp_to_enc (gcry_sexp_t sexp, gcry_mpi_t **retarray, size_t *retsize,
+ gcry_module_t *retalgo, int *ret_modern, int *flags,
+ struct pk_encoding_ctx *ctx)
{
gcry_err_code_t err = 0;
gcry_sexp_t list = NULL, l2 = NULL;
@@ -2442,7 +2441,8 @@ sexp_to_enc (gcry_sexp_t sexp, gcry_mpi_t **retarray, gcry_module_t *retalgo,
pubkey = (gcry_pk_spec_t *) module->spec;
elems = pubkey->elements_enc;
- array = gcry_calloc (strlen (elems) + 1, sizeof (*array));
+ *retsize = strlen (elems);
+ array = gcry_calloc (*retsize, sizeof (*array));
if (!array)
{
err = gpg_err_code_from_syserror ();
@@ -2929,13 +2929,16 @@ gcry_pk_encrypt (gcry_sexp_t *r_ciph, gcry_sexp_t s_data, gcry_sexp_t s_pkey)
gcry_err_code_t rc;
gcry_pk_spec_t *pubkey = NULL;
gcry_module_t module = NULL;
+ size_t pkey_size = 0;
+ size_t ciph_size = 0;
*r_ciph = NULL;
REGISTER_DEFAULT_PUBKEYS;
/* Get the key. */
- rc = sexp_to_key (s_pkey, 0, GCRY_PK_USAGE_ENCR, NULL, &pkey, &module, NULL);
+ rc = sexp_to_key (s_pkey, 0, GCRY_PK_USAGE_ENCR, NULL, &pkey, &pkey_size,
+ &module, NULL);
if (rc)
goto leave;
@@ -2960,7 +2963,8 @@ gcry_pk_encrypt (gcry_sexp_t *r_ciph, gcry_sexp_t s_data, gcry_sexp_t s_pkey)
goto leave;
/* Now we can encrypt DATA to CIPH. */
- ciph = gcry_calloc (strlen (algo_elems) + 1, sizeof (*ciph));
+ ciph_size = strlen (algo_elems);
+ ciph = gcry_calloc (ciph_size, sizeof (*ciph));
if (!ciph)
{
rc = gpg_err_code_from_syserror ();
@@ -3042,13 +3046,13 @@ gcry_pk_encrypt (gcry_sexp_t *r_ciph, gcry_sexp_t s_data, gcry_sexp_t s_pkey)
leave:
if (pkey)
{
- release_mpi_array (pkey);
+ release_mpi_array (pkey, pkey_size);
gcry_free (pkey);
}
if (ciph)
{
- release_mpi_array (ciph);
+ release_mpi_array (ciph, ciph_size);
gcry_free (ciph);
}
@@ -3102,6 +3106,8 @@ gcry_pk_decrypt (gcry_sexp_t *r_plain, gcry_sexp_t s_data, gcry_sexp_t s_skey)
struct pk_encoding_ctx ctx;
gcry_err_code_t rc;
gcry_module_t module_enc = NULL, module_key = NULL;
+ size_t skey_size = 0;
+ size_t data_size = 0;
*r_plain = NULL;
ctx.label = NULL;
@@ -3109,12 +3115,13 @@ gcry_pk_decrypt (gcry_sexp_t *r_plain, gcry_sexp_t s_data, gcry_sexp_t s_skey)
REGISTER_DEFAULT_PUBKEYS;
rc = sexp_to_key (s_skey, 1, GCRY_PK_USAGE_ENCR, NULL,
- &skey, &module_key, NULL);
+ &skey, &skey_size, &module_key, NULL);
if (rc)
goto leave;
init_encoding_ctx (&ctx, PUBKEY_OP_DECRYPT, gcry_pk_get_nbits (s_skey));
- rc = sexp_to_enc (s_data, &data, &module_enc, &modern, &flags, &ctx);
+ rc = sexp_to_enc (s_data, &data, &data_size, &module_enc, &modern,
+ &flags, &ctx);
if (rc)
goto leave;
@@ -3165,7 +3172,7 @@ gcry_pk_decrypt (gcry_sexp_t *r_plain, gcry_sexp_t s_data, gcry_sexp_t s_skey)
if (skey)
{
- release_mpi_array (skey);
+ release_mpi_array (skey, skey_size);
gcry_free (skey);
}
@@ -3173,7 +3180,7 @@ gcry_pk_decrypt (gcry_sexp_t *r_plain, gcry_sexp_t s_data, gcry_sexp_t s_skey)
if (data)
{
- release_mpi_array (data);
+ release_mpi_array (data, data_size);
gcry_free (data);
}
@@ -3233,13 +3240,15 @@ gcry_pk_sign (gcry_sexp_t *r_sig, gcry_sexp_t s_hash, gcry_sexp_t s_skey)
int i;
int is_ecc;
gcry_err_code_t rc;
+ size_t skey_size = 0;
+ size_t result_size = 0;
*r_sig = NULL;
REGISTER_DEFAULT_PUBKEYS;
rc = sexp_to_key (s_skey, 1, GCRY_PK_USAGE_SIGN, NULL,
- &skey, &module, &is_ecc);
+ &skey, &skey_size, &module, &is_ecc);
if (rc)
goto leave;
@@ -3260,7 +3269,8 @@ gcry_pk_sign (gcry_sexp_t *r_sig, gcry_sexp_t s_hash, gcry_sexp_t s_skey)
if (rc)
goto leave;
- result = gcry_calloc (strlen (algo_elems) + 1, sizeof (*result));
+ result_size = strlen (algo_elems);
+ result = gcry_calloc (result_size, sizeof (*result));
if (!result)
{
rc = gpg_err_code_from_syserror ();
@@ -3339,7 +3349,7 @@ gcry_pk_sign (gcry_sexp_t *r_sig, gcry_sexp_t s_hash, gcry_sexp_t s_skey)
leave:
if (skey)
{
- release_mpi_array (skey);
+ release_mpi_array (skey, skey_size);
gcry_free (skey);
}
@@ -3348,7 +3358,7 @@ gcry_pk_sign (gcry_sexp_t *r_sig, gcry_sexp_t s_hash, gcry_sexp_t s_skey)
if (result)
{
- release_mpi_array (result);
+ release_mpi_array (result, result_size);
gcry_free (result);
}
@@ -3370,15 +3380,17 @@ gcry_pk_verify (gcry_sexp_t s_sig, gcry_sexp_t s_hash, gcry_sexp_t s_pkey)
gcry_mpi_t *pkey = NULL, hash = NULL, *sig = NULL;
struct pk_encoding_ctx ctx;
gcry_err_code_t rc;
+ size_t pkey_size = 0;
+ size_t sig_size = 0;
REGISTER_DEFAULT_PUBKEYS;
rc = sexp_to_key (s_pkey, 0, GCRY_PK_USAGE_SIGN, NULL,
- &pkey, &module_key, NULL);
+ &pkey, &pkey_size, &module_key, NULL);
if (rc)
goto leave;
- rc = sexp_to_sig (s_sig, &sig, &module_sig);
+ rc = sexp_to_sig (s_sig, &sig, &sig_size, &module_sig);
if (rc)
goto leave;
@@ -3403,12 +3415,12 @@ gcry_pk_verify (gcry_sexp_t s_sig, gcry_sexp_t s_hash, gcry_sexp_t s_pkey)
leave:
if (pkey)
{
- release_mpi_array (pkey);
+ release_mpi_array (pkey, pkey_size);
gcry_free (pkey);
}
if (sig)
{
- release_mpi_array (sig);
+ release_mpi_array (sig, sig_size);
gcry_free (sig);
}
if (hash)
@@ -3443,15 +3455,16 @@ gcry_pk_testkey (gcry_sexp_t s_key)
gcry_module_t module = NULL;
gcry_mpi_t *key = NULL;
gcry_err_code_t rc;
+ size_t key_size = 0;
REGISTER_DEFAULT_PUBKEYS;
/* Note we currently support only secret key checking. */
- rc = sexp_to_key (s_key, 1, 0, NULL, &key, &module, NULL);
+ rc = sexp_to_key (s_key, 1, 0, NULL, &key, &key_size, &module, NULL);
if (! rc)
{
rc = pubkey_check_secret_key (module->mod_id, key);
- release_mpi_array (key);
+ release_mpi_array (key, key_size);
gcry_free (key);
}
return gcry_error (rc);
@@ -3511,6 +3524,8 @@ gcry_pk_genkey (gcry_sexp_t *r_key, gcry_sexp_t s_parms)
gcry_sexp_t extrainfo = NULL;
unsigned int nbits = 0;
unsigned long use_e = 0;
+ size_t skey_size = 0;
+ size_t factors_size = 0;
skey[0] = NULL;
*r_key = NULL;
@@ -3618,7 +3633,7 @@ gcry_pk_genkey (gcry_sexp_t *r_key, gcry_sexp_t s_parms)
/* Key generation succeeded: Build an S-expression. */
{
char *string, *p;
- size_t nelem=0, nelem_cp = 0, needed=0;
+ size_t nelem = 0, nelem_cp = 0, needed=0;
gcry_mpi_t mpis[30];
int percent_s_idx = -1;
@@ -3628,6 +3643,7 @@ gcry_pk_genkey (gcry_sexp_t *r_key, gcry_sexp_t s_parms)
{
for (i = 0; factors[i]; i++)
nelem++;
+ factors_size = i;
}
nelem_cp = nelem;
@@ -3674,8 +3690,7 @@ gcry_pk_genkey (gcry_sexp_t *r_key, gcry_sexp_t s_parms)
}
p = stpcpy (p, "))");
- /* Hack to make release_mpi_array() work. */
- skey[i] = NULL;
+ skey_size = i;
if (extrainfo && percent_s_idx == -1)
{
@@ -3736,12 +3751,12 @@ gcry_pk_genkey (gcry_sexp_t *r_key, gcry_sexp_t s_parms)
leave:
gcry_free (name);
gcry_sexp_release (extrainfo);
- release_mpi_array (skey);
+ release_mpi_array (skey, skey_size);
/* Don't free SKEY itself, it is an stack allocated array. */
if (factors)
{
- release_mpi_array ( factors );
+ release_mpi_array (factors, factors_size);
gcry_free (factors);
}
@@ -3773,6 +3788,7 @@ gcry_pk_get_nbits (gcry_sexp_t key)
gcry_mpi_t *keyarr = NULL;
unsigned int nbits = 0;
gcry_err_code_t rc;
+ size_t keyarr_size = 0;
REGISTER_DEFAULT_PUBKEYS;
@@ -3780,9 +3796,9 @@ gcry_pk_get_nbits (gcry_sexp_t key)
ECC we would only need to look at P and stop parsing right
away. */
- rc = sexp_to_key (key, 0, 0, NULL, &keyarr, &module, NULL);
+ rc = sexp_to_key (key, 0, 0, NULL, &keyarr, &keyarr_size, &module, NULL);
if (rc == GPG_ERR_INV_OBJ)
- rc = sexp_to_key (key, 1, 0, NULL, &keyarr, &module, NULL);
+ rc = sexp_to_key (key, 1, 0, NULL, &keyarr, &keyarr_size, &module, NULL);
if (rc)
return 0; /* Error - 0 is a suitable indication for that. */
@@ -3793,7 +3809,7 @@ gcry_pk_get_nbits (gcry_sexp_t key)
_gcry_module_release (module);
ath_mutex_unlock (&pubkeys_registered_lock);
- release_mpi_array (keyarr);
+ release_mpi_array (keyarr, keyarr_size);
gcry_free (keyarr);
return nbits;
@@ -3922,6 +3938,7 @@ gcry_pk_get_curve (gcry_sexp_t key, int iterator, unsigned int *r_nbits)
char *name = NULL;
const char *result = NULL;
int want_private = 1;
+ size_t pkey_size = 0;
if (r_nbits)
*r_nbits = 0;
@@ -3953,7 +3970,8 @@ gcry_pk_get_curve (gcry_sexp_t key, int iterator, unsigned int *r_nbits)
/* Get the key. We pass the names of the parameters for
override_elems; this allows to call this function without the
actual public key parameter. */
- if (sexp_to_key (key, want_private, 0, "pabgn", &pkey, &module, NULL))
+ if (sexp_to_key (key, want_private, 0, "pabgn", &pkey, &pkey_size,
+ &module, NULL))
goto leave;
}
else
@@ -3974,7 +3992,7 @@ gcry_pk_get_curve (gcry_sexp_t key, int iterator, unsigned int *r_nbits)
leave:
if (pkey)
{
- release_mpi_array (pkey);
+ release_mpi_array (pkey, pkey_size);
gcry_free (pkey);
}
if (module)
--
More information about the Gcrypt-devel
mailing list