Skip to content

Commit

Permalink
Fix various memory leaks. (#245)
Browse files Browse the repository at this point in the history 
This commit fixes various memory leaks that have been found using ASan.
  • Loading branch information
thb-sb authored Sep 7, 2023
1 parent 0e79d12 commit 2ef99c4
Show file tree
Hide file tree
Showing 5 changed files with 46 additions and 26 deletions.
1 change: 1 addition & 0 deletions oqsprov/oqs_decode_der2key.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,7 @@ OQSX_KEY *oqsx_d2i_PUBKEY(OQSX_KEY **a, const unsigned char **pp, long length)
xpk = oqsx_d2i_X509_PUBKEY_INTERNAL(pp, length, NULL);

key = oqsx_key_from_x509pubkey(xpk, NULL, NULL);
X509_PUBKEY_free(xpk);

if (key == NULL)
return NULL;
Expand Down
21 changes: 15 additions & 6 deletions oqsprov/oqsprov_keys.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -580,16 +580,21 @@ static int oqsx_hybsig_init(int bit_security, OQSX_EVP_CTX *evp_ctx,

if (idx < 3) { // EC
ret = EVP_PKEY_paramgen_init(evp_ctx->ctx);
ON_ERR_GOTO(ret <= 0, err);
ON_ERR_GOTO(ret <= 0, free_evp_ctx);

ret = EVP_PKEY_CTX_set_ec_paramgen_curve_nid(evp_ctx->ctx,
evp_ctx->evp_info->nid);
ON_ERR_GOTO(ret <= 0, err);
ON_ERR_GOTO(ret <= 0, free_evp_ctx);

ret = EVP_PKEY_paramgen(evp_ctx->ctx, &evp_ctx->keyParam);
ON_ERR_GOTO(ret <= 0 || !evp_ctx->keyParam, err);
ON_ERR_GOTO(ret <= 0 || !evp_ctx->keyParam, free_evp_ctx);
}
// RSA bit length set only during keygen
goto err;

free_evp_ctx:
EVP_PKEY_CTX_free(evp_ctx->ctx);
evp_ctx->ctx = NULL;

err:
return ret;
Expand Down Expand Up @@ -867,12 +872,14 @@ void oqsx_key_free(OQSX_KEY *key)
else if (key->keytype == KEY_TYPE_ECP_HYB_KEM
|| key->keytype == KEY_TYPE_ECX_HYB_KEM) {
OQS_KEM_free(key->oqsx_provider_ctx.oqsx_qs_ctx.kem);
} else
OQS_SIG_free(key->oqsx_provider_ctx.oqsx_qs_ctx.sig);
EVP_PKEY_free(key->classical_pkey);
if (key->oqsx_provider_ctx.oqsx_evp_ctx) {
EVP_PKEY_CTX_free(key->oqsx_provider_ctx.oqsx_evp_ctx->ctx);
EVP_PKEY_free(key->oqsx_provider_ctx.oqsx_evp_ctx->keyParam);
OPENSSL_free(key->oqsx_provider_ctx.oqsx_evp_ctx);
} else
OQS_SIG_free(key->oqsx_provider_ctx.oqsx_qs_ctx.sig);
OPENSSL_free(key->classical_pkey);
}
#ifdef OQS_PROVIDER_NOATOMIC
CRYPTO_THREAD_lock_free(key->lock);
#endif
Expand Down Expand Up @@ -1036,6 +1043,7 @@ static EVP_PKEY *oqsx_key_gen_evp_key(OQSX_EVP_CTX *ctx, unsigned char *pubkey,
EVP_PKEY *ck2 = d2i_PrivateKey(ctx->evp_info->keytype, NULL,
&privkey_enc2, privkeylen);
ON_ERR_SET_GOTO(!ck2, ret, -14, errhyb);
EVP_PKEY_free(ck2);
}
ENCODE_UINT32(pubkey, pubkeylen);
ENCODE_UINT32(privkey, privkeylen);
Expand Down Expand Up @@ -1087,6 +1095,7 @@ int oqsx_key_gen(OQSX_KEY *key)
ret = oqsx_key_gen_oqs(key, 0);
} else {
EVP_PKEY_free(pkey);
pkey = NULL;
ret = oqsx_key_gen_oqs(key, 1);
}
} else if (key->keytype == KEY_TYPE_SIG) {
Expand Down
35 changes: 20 additions & 15 deletions test/oqs_test_endecode.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,7 @@ static EVP_PKEY *oqstest_make_key(const char *type, EVP_PKEY *template,

static int encode_EVP_PKEY_prov(const EVP_PKEY *pkey, const char *format,
const char *structure, const char *pass,
const int selection, void **encoded,
long *encoded_len)
const int selection, BUF_MEM **encoded)
{
OSSL_ENCODER_CTX *ectx;
BIO *mem_ser = NULL;
Expand Down Expand Up @@ -110,8 +109,9 @@ static int encode_EVP_PKEY_prov(const EVP_PKEY *pkey, const char *format,
goto end;

/* pkey was successfully encoded into the bio */
*encoded = mem_buf->data;
*encoded_len = mem_buf->length;
*encoded = BUF_MEM_new();
(*encoded)->data = mem_buf->data;
(*encoded)->length = mem_buf->length;

/* Detach the encoded output */
mem_buf->data = NULL;
Expand Down Expand Up @@ -159,50 +159,55 @@ static int decode_EVP_PKEY_prov(const char *input_type, const char *structure,
pkey = NULL;

end:
EVP_PKEY_free(pkey);
BIO_free(encoded_bio);
OSSL_DECODER_CTX_free(dctx);
EVP_PKEY_free(pkey);
return ok;
}

static int test_oqs_encdec(const char *sigalg_name)
{
EVP_PKEY *pkey = NULL;
EVP_PKEY *decoded_pkey = NULL;
void *encoded = NULL;
long encoded_len = 0;
BUF_MEM *encoded = NULL;
size_t i;
int ok = 0;

for (i = 0; i < nelem(test_params_list); i++) {

pkey = oqstest_make_key(sigalg_name, NULL, NULL);
if (pkey == NULL)
goto end;

if (!encode_EVP_PKEY_prov(
pkey, test_params_list[i].format, test_params_list[i].structure,
test_params_list[i].pass, test_params_list[i].selection,
&encoded, &encoded_len)) {
if (!encode_EVP_PKEY_prov(pkey, test_params_list[i].format,
test_params_list[i].structure,
test_params_list[i].pass,
test_params_list[i].selection, &encoded)) {
printf("Failed encoding %s", sigalg_name);
goto end;
}
if (!decode_EVP_PKEY_prov(
test_params_list[i].format, test_params_list[i].structure,
test_params_list[i].pass, test_params_list[i].keytype,
test_params_list[i].selection, &decoded_pkey, encoded,
encoded_len)) {
test_params_list[i].selection, &decoded_pkey, encoded->data,
encoded->length)) {
printf("Failed decoding %s", sigalg_name);
goto end;
}

if (EVP_PKEY_eq(pkey, decoded_pkey) != 1)
goto end;
EVP_PKEY_free(pkey);
pkey = NULL;
EVP_PKEY_free(decoded_pkey);
decoded_pkey = NULL;
BUF_MEM_free(encoded);
encoded = NULL;
}
ok = 1;
end:
EVP_PKEY_free(pkey);
EVP_PKEY_free(decoded_pkey);
BUF_MEM_free(encoded);
return ok;
}

Expand Down Expand Up @@ -252,11 +257,11 @@ int main(int argc, char *argv[])
errcnt++;
}

OSSL_LIB_CTX_free(libctx);
OSSL_PROVIDER_unload(dfltprov);
OSSL_PROVIDER_unload(keyprov);
if (OPENSSL_VERSION_PREREQ(3, 1))
OSSL_PROVIDER_unload(oqsprov); // avoid crash in 3.0.x
OSSL_LIB_CTX_free(libctx);
OSSL_LIB_CTX_free(keyctx);

TEST_ASSERT(errcnt == 0)
Expand Down
11 changes: 8 additions & 3 deletions test/oqs_test_kems.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@ static int test_oqs_kems(const char *kemalg_name)
EVP_MD_CTX *mdctx = NULL;
EVP_PKEY_CTX *ctx = NULL;
EVP_PKEY *key = NULL;
unsigned char *out, *secenc, *secdec;
unsigned char *out = NULL;
unsigned char *secenc = NULL;
unsigned char *secdec = NULL;
size_t outlen, seclen;

int testresult = 1;
Expand All @@ -34,7 +36,7 @@ static int test_oqs_kems(const char *kemalg_name)

if (!testresult)
goto err;
OPENSSL_free(ctx);
EVP_PKEY_CTX_free(ctx);
ctx = NULL;

testresult
Expand Down Expand Up @@ -64,7 +66,10 @@ static int test_oqs_kems(const char *kemalg_name)

err:
EVP_PKEY_free(key);
OPENSSL_free(ctx);
EVP_PKEY_CTX_free(ctx);
OPENSSL_free(out);
OPENSSL_free(secenc);
OPENSSL_free(secdec);
return testresult;
}

Expand Down
4 changes: 2 additions & 2 deletions test/oqs_test_signatures.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ static int test_oqs_signatures(const char *sigalg_name)

EVP_MD_CTX_free(mdctx);
EVP_PKEY_free(key);
OPENSSL_free(ctx);
EVP_PKEY_CTX_free(ctx);
OPENSSL_free(sig);
mdctx = NULL;
key = NULL;
Expand All @@ -84,7 +84,7 @@ static int test_oqs_signatures(const char *sigalg_name)

EVP_MD_CTX_free(mdctx);
EVP_PKEY_free(key);
OPENSSL_free(ctx);
EVP_PKEY_CTX_free(ctx);
OPENSSL_free(sig);
return testresult;
}
Expand Down

0 comments on commit 2ef99c4

Please sign in to comment.