From b31f9de17f0dba00b9b2d6cdbd122a243fd0cf96 Mon Sep 17 00:00:00 2001 From: Felipe Ventura Date: Mon, 22 Jan 2024 13:32:33 -0600 Subject: [PATCH] solved memory leaks --- oqsprov/oqs_encode_key2any.c | 87 +++++++++++++++++++----------------- oqsprov/oqsprov_keys.c | 19 +++++--- 2 files changed, 57 insertions(+), 49 deletions(-) diff --git a/oqsprov/oqs_encode_key2any.c b/oqsprov/oqs_encode_key2any.c index d54fd6ca..7d8851fe 100644 --- a/oqsprov/oqs_encode_key2any.c +++ b/oqsprov/oqs_encode_key2any.c @@ -500,7 +500,8 @@ static int oqsx_spki_pub_to_der(const void *vxkey, unsigned char **pder) { const OQSX_KEY *oqsxkey = vxkey; unsigned char *keyblob, *buf; - int keybloblen, nid; + int keybloblen, nid, buflen = 0; + ASN1_OCTET_STRING oct; STACK_OF(ASN1_TYPE) *sk = NULL; int ret = 0; @@ -542,55 +543,55 @@ static int oqsx_spki_pub_to_der(const void *vxkey, unsigned char **pder) } #endif } else { - ASN1_TYPE **aType - = OPENSSL_malloc(oqsxkey->numkeys * sizeof(ASN1_TYPE)); - ASN1_STRING **aString - = OPENSSL_malloc(oqsxkey->numkeys * sizeof(ASN1_STRING)); - ASN1_STRING **tempOct - = OPENSSL_malloc(oqsxkey->numkeys * sizeof(ASN1_STRING)); - unsigned char **temp - = OPENSSL_malloc(oqsxkey->numkeys * sizeof(void *)); - unsigned char **cbuf - = OPENSSL_malloc(oqsxkey->numkeys * sizeof(void *)); - int len, i; if ((sk = sk_ASN1_TYPE_new_null()) == NULL) return -1; + ASN1_TYPE *aType[oqsxkey->numkeys]; + ASN1_OCTET_STRING *aString[oqsxkey->numkeys]; + unsigned char *temp[oqsxkey->numkeys]; + size_t templen[oqsxkey->numkeys]; + int i; for (i = 0; i < oqsxkey->numkeys; i++) { aType[i] = ASN1_TYPE_new(); aString[i] = ASN1_OCTET_STRING_new(); - tempOct[i] = ASN1_OCTET_STRING_new(); temp[i] = NULL; - len = oqsxkey->pubkeylen_cmp[i]; - cbuf[i] = OPENSSL_memdup(oqsxkey->comp_pubkey[i], len); - ASN1_STRING_set0(tempOct[i], cbuf[i], len); - keybloblen = i2d_ASN1_OCTET_STRING(tempOct[i], &temp[i]); - ASN1_STRING_set0(aString[i], temp[i], keybloblen); - ASN1_TYPE_set(aType[i], V_ASN1_SEQUENCE, aString[i]); + buflen = oqsxkey->pubkeylen_cmp[i]; + buf = OPENSSL_secure_malloc(buflen); + memcpy(buf, oqsxkey->comp_pubkey[i], buflen); + + oct.data = buf; + oct.length = buflen; + templen[i] = i2d_ASN1_OCTET_STRING(&oct, &temp[i]); + ASN1_STRING_set(aString[i], temp[i], templen[i]); + ASN1_TYPE_set1(aType[i], V_ASN1_SEQUENCE, aString[i]); if (!sk_ASN1_TYPE_push(sk, aType[i])) { - for (i = 0; i < oqsxkey->numkeys; i++) { - OPENSSL_free(temp[i]); - OPENSSL_free(cbuf[i]); - OPENSSL_free(aType[i]); - OPENSSL_free(aString[i]); - OPENSSL_free(tempOct[i]); + for (int j = 0; j <= i; j++) { + OPENSSL_cleanse(aString[j]->data, aString[j]->length); + ASN1_OCTET_STRING_free(aString[j]); + OPENSSL_cleanse(aType[j]->value.sequence->data, + aType[j]->value.sequence->length); + OPENSSL_clear_free(temp[j], templen[j]); } - OPENSSL_free(sk); + + sk_ASN1_TYPE_pop_free(sk, &ASN1_TYPE_free); + OPENSSL_secure_clear_free(buf, buflen); return -1; } + OPENSSL_secure_clear_free(buf, buflen); } keybloblen = i2d_ASN1_SEQUENCE_ANY(sk, pder); for (i = 0; i < oqsxkey->numkeys; i++) { - OPENSSL_free(temp[i]); - OPENSSL_free(cbuf[i]); - OPENSSL_free(aType[i]); - OPENSSL_free(aString[i]); - OPENSSL_free(tempOct[i]); + OPENSSL_cleanse(aString[i]->data, aString[i]->length); + ASN1_OCTET_STRING_free(aString[i]); + OPENSSL_cleanse(aType[i]->value.sequence->data, + aType[i]->value.sequence->length); + OPENSSL_clear_free(temp[i], templen[i]); } - OPENSSL_free(sk); + + sk_ASN1_TYPE_pop_free(sk, &ASN1_TYPE_free); return keybloblen; } @@ -694,14 +695,12 @@ static int oqsx_pki_priv_to_der(const void *vxkey, unsigned char **pder) keybloblen = 0; // signal error } } else { - ASN1_TYPE **aType - = OPENSSL_malloc(oqsxkey->numkeys * sizeof(ASN1_TYPE)); - ASN1_OCTET_STRING **aString - = OPENSSL_malloc(oqsxkey->numkeys * sizeof(ASN1_OCTET_STRING)); - unsigned char **temp - = OPENSSL_secure_malloc(oqsxkey->numkeys * sizeof(void *)); + ASN1_TYPE *aType[oqsxkey->numkeys]; + ASN1_OCTET_STRING *aString[oqsxkey->numkeys]; + unsigned char *temp[oqsxkey->numkeys]; size_t templen[oqsxkey->numkeys]; int i; + if ((sk = sk_ASN1_TYPE_new_null()) == NULL) return -1; @@ -717,7 +716,8 @@ static int oqsx_pki_priv_to_der(const void *vxkey, unsigned char **pder) ASN1_OCTET_STRING_free(aString[j]); OPENSSL_cleanse(aType[j]->value.sequence->data, aType[j]->value.sequence->length); - OPENSSL_clear_free(temp[j], templen[j]); + if (j < i) + OPENSSL_clear_free(temp[j], templen[j]); } if (sk_ASN1_TYPE_num(sk) != -1) @@ -745,7 +745,8 @@ static int oqsx_pki_priv_to_der(const void *vxkey, unsigned char **pder) ASN1_OCTET_STRING_free(aString[j]); OPENSSL_cleanse(aType[j]->value.sequence->data, aType[j]->value.sequence->length); - OPENSSL_clear_free(temp[j], templen[j]); + if (j < i) + OPENSSL_clear_free(temp[j], templen[j]); } if (sk_ASN1_TYPE_num(sk) != -1) @@ -792,7 +793,9 @@ static int oqsx_pki_priv_to_der(const void *vxkey, unsigned char **pder) return -1; } OPENSSL_free(name); - if (i + 1 < oqsxkey->numkeys){ // clear buf and oct if is not the last call + if (i + 1 + < oqsxkey + ->numkeys) { // clear buf and oct if is not the last call OPENSSL_secure_clear_free(buf, buflen); } } @@ -805,7 +808,7 @@ static int oqsx_pki_priv_to_der(const void *vxkey, unsigned char **pder) aType[i]->value.sequence->length); OPENSSL_clear_free(temp[i], templen[i]); } - + sk_ASN1_TYPE_pop_free(sk, &ASN1_TYPE_free); } OPENSSL_secure_clear_free(buf, buflen); diff --git a/oqsprov/oqsprov_keys.c b/oqsprov/oqsprov_keys.c index bb074c6c..57080102 100644 --- a/oqsprov/oqsprov_keys.c +++ b/oqsprov/oqsprov_keys.c @@ -1030,11 +1030,12 @@ OQSX_KEY *oqsx_key_from_x509pubkey(const X509_PUBKEY *xpk, OSSL_LIB_CTX *libctx, if (get_keytype(OBJ_obj2nid(palg->algorithm)) == KEY_TYPE_CMP_SIG) { sk = d2i_ASN1_SEQUENCE_ANY(NULL, &p, plen); if (sk == NULL) { + sk_ASN1_TYPE_pop_free(sk, &ASN1_TYPE_free); ERR_raise(ERR_LIB_USER, OQSPROV_R_INVALID_ENCODING); return NULL; } else { count = sk_ASN1_TYPE_num(sk); - concat_key = OPENSSL_zalloc(plen); + concat_key = OPENSSL_zalloc(plen); aux = 0; for (i = 0; i < count; i++) { @@ -1042,16 +1043,18 @@ OQSX_KEY *oqsx_key_from_x509pubkey(const X509_PUBKEY *xpk, OSSL_LIB_CTX *libctx, buf = aType->value.sequence->data; buflen = aType->value.sequence->length; aux += buflen; - memcpy(concat_key + plen - 1 - aux , buf, buflen); + memcpy(concat_key + plen - 1 - aux, buf, buflen); + ASN1_TYPE_free(aType); } - p = OPENSSL_memdup(concat_key + plen - 1 - aux, aux); - OPENSSL_clear_free(concat_key, plen); + p = OPENSSL_memdup(concat_key + plen - 1 - aux, aux); + OPENSSL_clear_free(concat_key, plen); plen = aux; + sk_ASN1_TYPE_free(sk); } } oqsx = oqsx_key_op(palg, p, plen, KEY_OP_PUBLIC, libctx, propq); - if (get_keytype(OBJ_obj2nid(palg->algorithm)) == KEY_TYPE_CMP_SIG) + if (get_keytype(OBJ_obj2nid(palg->algorithm)) == KEY_TYPE_CMP_SIG) OPENSSL_clear_free(p, plen); return oqsx; } @@ -1085,6 +1088,7 @@ OQSX_KEY *oqsx_key_from_pkcs8(const PKCS8_PRIV_KEY_INFO *p8inf, } else { sk = d2i_ASN1_SEQUENCE_ANY(NULL, &p, plen); if (sk == NULL) { + sk_ASN1_TYPE_pop_free(sk, &ASN1_TYPE_free); ERR_raise(ERR_LIB_USER, OQSPROV_R_INVALID_ENCODING); return NULL; } else { @@ -1116,10 +1120,11 @@ OQSX_KEY *oqsx_key_from_pkcs8(const PKCS8_PRIV_KEY_INFO *p8inf, rsa_diff = nids_sig[6].length_private_key - buflen; } OPENSSL_free(name); + ASN1_TYPE_free(aType); } p = OPENSSL_memdup(concat_key + plen - 1 - aux, aux); - OPENSSL_clear_free(concat_key, plen); + OPENSSL_clear_free(concat_key, plen); plen = aux; sk_ASN1_TYPE_free(sk); } @@ -1128,7 +1133,7 @@ OQSX_KEY *oqsx_key_from_pkcs8(const PKCS8_PRIV_KEY_INFO *p8inf, oqsx = oqsx_key_op(palg, p, plen + rsa_diff, KEY_OP_PRIVATE, libctx, propq); if (get_keytype(OBJ_obj2nid(palg->algorithm)) != KEY_TYPE_CMP_SIG) { ASN1_OCTET_STRING_free(oct); - }else{ + } else { OPENSSL_clear_free(p, plen); } return oqsx;