Skip to content

Commit

Permalink
solved some memleaks
Browse files Browse the repository at this point in the history 
Signed-off-by: Felipe Ventura <[email protected]>
  • Loading branch information
@feventura
feventura committed Mar 12, 2024
1 parent 92b05a0 commit 9264e1f
Show file tree
Hide file tree
Showing 2 changed files with 74 additions and 50 deletions.
97 changes: 58 additions & 39 deletions oqsprov/oqs_encode_key2any.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -696,34 +696,36 @@ static int oqsx_pki_priv_to_der(const void *vxkey, unsigned char **pder)
} 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));
ASN1_OCTET_STRING **aString
= OPENSSL_malloc(oqsxkey->numkeys * sizeof(ASN1_OCTET_STRING));
unsigned char **temp
= OPENSSL_malloc(oqsxkey->numkeys * sizeof(void *));
unsigned char **cbuf
= OPENSSL_malloc(oqsxkey->numkeys * sizeof(void *));
= OPENSSL_secure_malloc(oqsxkey->numkeys * sizeof(void *));
size_t templen[oqsxkey->numkeys];
int i;
if ((sk = sk_ASN1_TYPE_new_null()) == NULL)
return -1;

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;
buflen = 0;

if ((name = get_cmpname(OBJ_sn2nid(oqsxkey->tls_name), i))
== NULL) {
OPENSSL_free(name);
for (i = 0; i < oqsxkey->numkeys; i++) {
ASN1_OCTET_STRING_free(aString[i]);
ASN1_OCTET_STRING_free(tempOct[i]);
ASN1_TYPE_free(aType[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);

if (sk_ASN1_TYPE_num(sk) != -1)
sk_ASN1_TYPE_pop_free(sk, &ASN1_TYPE_free);
else
ASN1_TYPE_free(aType[i]);

OPENSSL_free(name);
return -1;
}

Expand All @@ -737,57 +739,74 @@ static int oqsx_pki_priv_to_der(const void *vxkey, unsigned char **pder)
buflen += 4;
OPENSSL_free(enc_len);
if (buflen > oqsxkey->privkeylen_cmp[i]) {
OPENSSL_free(name);
for (i = 0; i < oqsxkey->numkeys; i++) {
ASN1_OCTET_STRING_free(aString[i]);
ASN1_OCTET_STRING_free(tempOct[i]);
ASN1_TYPE_free(aType[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);
ERR_raise(ERR_LIB_USER, OQSPROV_R_INVALID_ENCODING);

if (sk_ASN1_TYPE_num(sk) != -1)
sk_ASN1_TYPE_pop_free(sk, &ASN1_TYPE_free);
else
ASN1_TYPE_free(aType[i]);

OPENSSL_free(name);
return -1;
}
} else
buflen = oqsxkey->privkeylen_cmp[i];
} else
buflen = oqsxkey->privkeylen_cmp[i] + oqsxkey->pubkeylen_cmp[i];

cbuf[i] = OPENSSL_malloc(buflen);
buf = OPENSSL_secure_malloc(buflen);
if (get_oqsname_fromtls(name)
!= 0) { // include pubkey in privkey for PQC
memcpy(cbuf[i], oqsxkey->comp_privkey[i],
memcpy(buf, oqsxkey->comp_privkey[i],
oqsxkey->privkeylen_cmp[i]);
memcpy(cbuf[i] + oqsxkey->privkeylen_cmp[i],
memcpy(buf + oqsxkey->privkeylen_cmp[i],
oqsxkey->comp_pubkey[i], oqsxkey->pubkeylen_cmp[i]);
} else
memcpy(cbuf[i], oqsxkey->comp_privkey[i], buflen);
memcpy(buf, oqsxkey->comp_privkey[i], buflen);

ASN1_STRING_set0(tempOct[i], cbuf[i], buflen);
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]);
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++) {
ASN1_OCTET_STRING_free(aString[i]);
ASN1_OCTET_STRING_free(tempOct[i]);
ASN1_TYPE_free(aType[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_free(name);
OPENSSL_secure_clear_free(buf, buflen);
return -1;
}
OPENSSL_free(name);
if (i + 1 < oqsxkey->numkeys){ // clear buf and oct if is not the last call
OPENSSL_secure_clear_free(buf, buflen);
}
}
keybloblen = i2d_ASN1_SEQUENCE_ANY(sk, pder);

for (i = 0; i < oqsxkey->numkeys; i++) {
OPENSSL_cleanse(aString[i]->data, aString[i]->length);
ASN1_OCTET_STRING_free(aString[i]);
ASN1_OCTET_STRING_free(tempOct[i]);
ASN1_TYPE_free(aType[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);
}
OPENSSL_secure_clear_free(buf, buflen);
return keybloblen;
Expand Down
27 changes: 16 additions & 11 deletions oqsprov/oqsprov_keys.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1020,7 +1020,6 @@ OQSX_KEY *oqsx_key_from_x509pubkey(const X509_PUBKEY *xpk, OSSL_LIB_CTX *libctx,
STACK_OF(ASN1_TYPE) *sk = NULL;
ASN1_TYPE *aType = NULL;
ASN1_OCTET_STRING *oct = NULL;
X509_PUBKEY *p8info_buf = X509_PUBKEY_new();
const unsigned char *buf;
unsigned char *concat_key;
int count, aux, i, buflen;
Expand All @@ -1035,24 +1034,25 @@ OQSX_KEY *oqsx_key_from_x509pubkey(const X509_PUBKEY *xpk, OSSL_LIB_CTX *libctx,
return NULL;
} else {
count = sk_ASN1_TYPE_num(sk);
concat_key = OPENSSL_secure_malloc(plen);
concat_key = OPENSSL_zalloc(plen);

aux = 0;
for (i = 0; i < count; i++) {
aType = sk_ASN1_TYPE_pop(sk);
buf = aType->value.sequence->data;
buflen = aType->value.sequence->length;
aux += buflen;
memcpy(concat_key + plen - aux, buf, buflen);
memcpy(concat_key + plen - 1 - aux , buf, buflen);
}

p = OPENSSL_memdup(concat_key + plen - aux, aux);
p = OPENSSL_memdup(concat_key + plen - 1 - aux, aux);
OPENSSL_clear_free(concat_key, plen);
plen = aux;
OPENSSL_free(concat_key);
}
}
oqsx = oqsx_key_op(palg, p, plen, KEY_OP_PUBLIC, libctx, propq);

if (get_keytype(OBJ_obj2nid(palg->algorithm)) == KEY_TYPE_CMP_SIG)
OPENSSL_clear_free(p, plen);
return oqsx;
}

Expand All @@ -1069,7 +1069,6 @@ OQSX_KEY *oqsx_key_from_pkcs8(const PKCS8_PRIV_KEY_INFO *p8inf,
const unsigned char *buf;
unsigned char *concat_key;
int count, aux, i, buflen, rsa_diff = 0;
PKCS8_PRIV_KEY_INFO *p8info_buf = PKCS8_PRIV_KEY_INFO_new();

if (!PKCS8_pkey_get0(NULL, &p, &plen, &palg, p8inf))
return 0;
Expand All @@ -1090,7 +1089,7 @@ OQSX_KEY *oqsx_key_from_pkcs8(const PKCS8_PRIV_KEY_INFO *p8inf,
return NULL;
} else {
count = sk_ASN1_TYPE_num(sk);
concat_key = OPENSSL_secure_malloc(plen);
concat_key = OPENSSL_zalloc(plen);

aux = 0;
for (i = 0; i < count; i++) {
Expand All @@ -1106,7 +1105,7 @@ OQSX_KEY *oqsx_key_from_pkcs8(const PKCS8_PRIV_KEY_INFO *p8inf,
buf = aType->value.sequence->data;
buflen = aType->value.sequence->length;
aux += buflen;
memcpy(concat_key + plen - aux, buf, buflen);
memcpy(concat_key + plen - 1 - aux, buf, buflen);
// if is a RSA key the actual encoding size might be different
// from max size we calculate that difference for to facilitate
// the key reconstruction
Expand All @@ -1119,13 +1118,19 @@ OQSX_KEY *oqsx_key_from_pkcs8(const PKCS8_PRIV_KEY_INFO *p8inf,
OPENSSL_free(name);
}

p = concat_key + plen - aux;
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 + rsa_diff, KEY_OP_PRIVATE, libctx, propq);
ASN1_OCTET_STRING_free(oct);
if (get_keytype(OBJ_obj2nid(palg->algorithm)) != KEY_TYPE_CMP_SIG) {
ASN1_OCTET_STRING_free(oct);
}else{
OPENSSL_clear_free(p, plen);
}
return oqsx;
}

Expand Down

0 comments on commit 9264e1f

Please sign in to comment.