From c0c0104a777286a622d7bc713bac7b9dac3c91fe Mon Sep 17 00:00:00 2001 From: Norman Ashley Date: Thu, 29 Aug 2024 01:49:27 -0400 Subject: [PATCH 1/4] Fix various static analysis issues. Signed-off-by: Norman Ashley --- oqsprov/oqs_encode_key2any.c | 11 ++++++++++- oqsprov/oqs_kem.c | 21 +++++++++------------ oqsprov/oqs_kmgmt.c | 2 +- oqsprov/oqsprov_keys.c | 10 ++++++---- test/oqs_test_evp_pkey_params.c | 2 ++ 5 files changed, 28 insertions(+), 18 deletions(-) diff --git a/oqsprov/oqs_encode_key2any.c b/oqsprov/oqs_encode_key2any.c index a1395325..ca9b571d 100644 --- a/oqsprov/oqs_encode_key2any.c +++ b/oqsprov/oqs_encode_key2any.c @@ -662,10 +662,19 @@ static int oqsx_pki_priv_to_der(const void *vxkey, unsigned char **pder) { OPENSSL_malloc(oqsxkey->numkeys * sizeof(unsigned char *)); size_t *templen = OPENSSL_malloc(oqsxkey->numkeys * sizeof(size_t)); PKCS8_PRIV_KEY_INFO *p8inf_internal = NULL; + sk = sk_ASN1_TYPE_new_null(); int i; - if ((sk = sk_ASN1_TYPE_new_null()) == NULL) + if (!sk || !templen || !aType || !aString || !temp) { + OPENSSL_free(aType); + OPENSSL_free(aString); + OPENSSL_free(temp); + OPENSSL_free(templen); + if (sk) { + sk_ASN1_TYPE_pop_free(sk, ASN1_TYPE_free); + } return -1; + } for (i = 0; i < oqsxkey->numkeys; i++) { aType[i] = ASN1_TYPE_new(); diff --git a/oqsprov/oqs_kem.c b/oqsprov/oqs_kem.c index 84ac17d0..adc11f46 100644 --- a/oqsprov/oqs_kem.c +++ b/oqsprov/oqs_kem.c @@ -115,18 +115,6 @@ static int oqs_qs_kem_encaps_keyslot(void *vpkemctx, unsigned char *out, OQS_KEM_PRINTF("OQS Warning: public key is NULL\n"); return -1; } - if (out == NULL || secret == NULL) { - if (outlen != NULL) { - *outlen = kem_ctx->length_ciphertext; - } - if (secretlen != NULL) { - *secretlen = kem_ctx->length_shared_secret; - } - OQS_KEM_PRINTF3("KEM returning lengths %ld and %ld\n", - kem_ctx->length_ciphertext, - kem_ctx->length_shared_secret); - return 1; - } if (outlen == NULL) { OQS_KEM_PRINTF("OQS Warning: outlen is NULL\n"); return -1; @@ -135,6 +123,15 @@ static int oqs_qs_kem_encaps_keyslot(void *vpkemctx, unsigned char *out, OQS_KEM_PRINTF("OQS Warning: secretlen is NULL\n"); return -1; } + if (out == NULL || secret == NULL) { + *outlen = kem_ctx->length_ciphertext; + *secretlen = kem_ctx->length_shared_secret; + OQS_KEM_PRINTF3("KEM returning lengths %ld and %ld\n", + kem_ctx->length_ciphertext, + kem_ctx->length_shared_secret); + return 1; + } + if (*outlen < kem_ctx->length_ciphertext) { OQS_KEM_PRINTF("OQS Warning: out buffer too small\n"); return -1; diff --git a/oqsprov/oqs_kmgmt.c b/oqsprov/oqs_kmgmt.c index c24ccaaf..1480abbc 100644 --- a/oqsprov/oqs_kmgmt.c +++ b/oqsprov/oqs_kmgmt.c @@ -376,7 +376,7 @@ static int oqsx_get_hybrid_params(OQSX_KEY *key, OSSL_PARAM params[]) { DECODE_UINT32(classical_privkey_len, key->privkey); } - if (key->comp_pubkey[1] != NULL) { + if (key->comp_pubkey && key->comp_pubkey[1] != NULL) { pq_pubkey = key->comp_pubkey[1]; pq_pubkey_len = key->pubkeylen - classical_pubkey_len - SIZE_OF_UINT32; } diff --git a/oqsprov/oqsprov_keys.c b/oqsprov/oqsprov_keys.c index d711ff56..173a3120 100644 --- a/oqsprov/oqsprov_keys.c +++ b/oqsprov/oqsprov_keys.c @@ -1497,10 +1497,12 @@ OQSX_KEY *oqsx_key_new(OSSL_LIB_CTX *libctx, char *oqs_name, char *tls_name, if (ret->lock) CRYPTO_THREAD_lock_free(ret->lock); #endif - OPENSSL_free(ret->tls_name); - OPENSSL_free(ret->propq); - OPENSSL_free(ret->comp_privkey); - OPENSSL_free(ret->comp_pubkey); + if (ret) { + OPENSSL_free(ret->tls_name); + OPENSSL_free(ret->propq); + OPENSSL_free(ret->comp_privkey); + OPENSSL_free(ret->comp_pubkey); + } OPENSSL_free(ret); return NULL; } diff --git a/test/oqs_test_evp_pkey_params.c b/test/oqs_test_evp_pkey_params.c index 18ac883a..b85f9cd1 100644 --- a/test/oqs_test_evp_pkey_params.c +++ b/test/oqs_test_evp_pkey_params.c @@ -534,6 +534,7 @@ int main(int argc, char **argv) { fprintf(stderr, cRED " No signature algorithms found" cNORM "\n"); ERR_print_errors_fp(stderr); ++errcnt; + goto next_alg; } for (; algs->algorithm_names != NULL; ++algs) { @@ -550,6 +551,7 @@ int main(int argc, char **argv) { } } +next_alg: algs = OSSL_PROVIDER_query_operation(oqs_provider, OSSL_OP_KEM, &query_nocache); if (!algs) { From e17e84be81aa078d18ef6da645c6908542925a20 Mon Sep 17 00:00:00 2001 From: Norman Ashley Date: Sat, 31 Aug 2024 15:06:39 -0400 Subject: [PATCH 2/4] Fix Dereference before null check. Signed-off-by: Norman Ashley --- oqsprov/oqs_kem.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/oqsprov/oqs_kem.c b/oqsprov/oqs_kem.c index adc11f46..aba8c073 100644 --- a/oqsprov/oqs_kem.c +++ b/oqsprov/oqs_kem.c @@ -103,13 +103,15 @@ static int oqs_qs_kem_encaps_keyslot(void *vpkemctx, unsigned char *out, size_t *outlen, unsigned char *secret, size_t *secretlen, int keyslot) { const PROV_OQSKEM_CTX *pkemctx = (PROV_OQSKEM_CTX *)vpkemctx; - const OQS_KEM *kem_ctx = pkemctx->kem->oqsx_provider_ctx.oqsx_qs_ctx.kem; + const OQS_KEM *kem_ctx = NULL; OQS_KEM_PRINTF("OQS KEM provider called: encaps\n"); - if (pkemctx->kem == NULL) { + if (!pkemctx->kem || !pkemctx->kem->oqsx_provider_ctx) { OQS_KEM_PRINTF("OQS Warning: OQS_KEM not initialized\n"); return -1; } + + kem_ctx = pkemctx->kem->oqsx_provider_ctx.oqsx_qs_ctx.kem; if (pkemctx->kem->comp_pubkey == NULL || pkemctx->kem->comp_pubkey[keyslot] == NULL) { OQS_KEM_PRINTF("OQS Warning: public key is NULL\n"); From afc0af608d95024008860cd541f15c3133116957 Mon Sep 17 00:00:00 2001 From: Norman Ashley Date: Sat, 31 Aug 2024 15:30:18 -0400 Subject: [PATCH 3/4] Fix Dereference before null check. Signed-off-by: Norman Ashley --- oqsprov/oqs_kem.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/oqsprov/oqs_kem.c b/oqsprov/oqs_kem.c index aba8c073..0ed3fbe4 100644 --- a/oqsprov/oqs_kem.c +++ b/oqsprov/oqs_kem.c @@ -106,7 +106,7 @@ static int oqs_qs_kem_encaps_keyslot(void *vpkemctx, unsigned char *out, const OQS_KEM *kem_ctx = NULL; OQS_KEM_PRINTF("OQS KEM provider called: encaps\n"); - if (!pkemctx->kem || !pkemctx->kem->oqsx_provider_ctx) { + if (pkemctx->kem == NULL) { OQS_KEM_PRINTF("OQS Warning: OQS_KEM not initialized\n"); return -1; } @@ -153,13 +153,14 @@ static int oqs_qs_kem_decaps_keyslot(void *vpkemctx, unsigned char *out, size_t *outlen, const unsigned char *in, size_t inlen, int keyslot) { const PROV_OQSKEM_CTX *pkemctx = (PROV_OQSKEM_CTX *)vpkemctx; - const OQS_KEM *kem_ctx = pkemctx->kem->oqsx_provider_ctx.oqsx_qs_ctx.kem; + const OQS_KEM *kem_ctx = NULL; OQS_KEM_PRINTF("OQS KEM provider called: decaps\n"); if (pkemctx->kem == NULL) { OQS_KEM_PRINTF("OQS Warning: OQS_KEM not initialized\n"); return -1; } + kem_ctx = pkemctx->kem->oqsx_provider_ctx.oqsx_qs_ctx.kem; if (pkemctx->kem->comp_privkey == NULL || pkemctx->kem->comp_privkey[keyslot] == NULL) { OQS_KEM_PRINTF("OQS Warning: private key is NULL\n"); From c0eae3c65f5b858d0930277fca94cb538498de32 Mon Sep 17 00:00:00 2001 From: Norman Ashley Date: Thu, 12 Sep 2024 20:04:28 -0400 Subject: [PATCH 4/4] Fixed deref without null check. Signed-off-by: Norman Ashley --- oqsprov/oqsprov.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/oqsprov/oqsprov.c b/oqsprov/oqsprov.c index b95a1741..ac5336f4 100644 --- a/oqsprov/oqsprov.c +++ b/oqsprov/oqsprov.c @@ -1164,7 +1164,7 @@ int OQS_PROVIDER_ENTRYPOINT_NAME(const OSSL_CORE_HANDLE *handle, * Not testing for errors is intentional. * At least one core version hangs up; so don't do this there: */ - if (strcmp("3.1.0", ossl_versionp)) { + if (ossl_versionp && strcmp("3.1.0", ossl_versionp)) { ERR_set_mark(); OBJ_create(oqs_oid_alg_list[i], oqs_oid_alg_list[i + 1], oqs_oid_alg_list[i + 1]);