Merge pull request #521 from ashman-p/na_81_rsc_leak-2

Address some Static Analysis Issues #519
open-quantum-safe · Sep 18, 2024 · 0312c00 · 0312c00
2 parents 4db09a9 + c0eae3c
commit 0312c00
Show file tree

Hide file tree

Showing 6 changed files with 34 additions and 21 deletions.
diff --git 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
@@ -103,30 +103,20 @@ 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) {
         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");
         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 +125,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;
@@ -154,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");

diff --git 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.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]);

diff --git 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
@@ -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) {