Hi list,
based on recent coverity scan I dug uout a few problems related to memory management, which would be great to have fixed:


 * subprimebits_attr in usr/lib/pkcs11/common/dp_obj.c

In function dp_x9dh_set_default_attributes, there is unused or not-cleaned up variable subprimebits_attr, which was reported by coverity.

The variable is initialized, but then not added into template, what is probably expected behavior. If not, it can be removed. My best guess is updating tmpl attributes using this variable (opencryptoki_subprimebits_attr.patch).


 * type_attr in usr/lib/pkcs11/common/key.c

Variable is not cleaned if some of the variables fails to malloc (opencryptoki_type_attr.patch).


 * apiSessp in usr/lib/pkcs11/api/api_interface.c

Allocated variable is not freed, when ST_OpenSession is zero (opencryptoki_apiSessp.patch).


 * ctx in usr/lib/pkcs11/common/key_mgr.c

Allocated variable ctx is not free when encr_mgr_init fails (opencryptoki_ctx.patch).


 * tmpl and a2 variables in usr/lib/pkcs11/common/template.c

This is more complicated and painful issue because of hard-to-get indentation, but my best guess is in last patch (opencryptoki_template.patch)


Best regards,

--
Jakub Jelen
Security Technologies
Red Hat

diff --git a/usr/lib/pkcs11/common/dp_obj.c b/usr/lib/pkcs11/common/dp_obj.c
index 14784fe..573ffab 100644
--- a/usr/lib/pkcs11/common/dp_obj.c
+++ b/usr/lib/pkcs11/common/dp_obj.c
@@ -784,6 +784,7 @@ dp_x9dh_set_default_attributes( TEMPLATE *tmpl, CK_ULONG mode )
    template_update_attribute( tmpl, subprime_attr );
    template_update_attribute( tmpl, base_attr );
    template_update_attribute( tmpl, primebits_attr );
+   template_update_attribute( tmpl, subprimebits_attr );
    template_update_attribute( tmpl, type_attr );
 
    return CKR_OK;
diff --git a/usr/lib/pkcs11/common/key.c b/usr/lib/pkcs11/common/key.c
index a9bc7a0..b6f7a86 100755
--- a/usr/lib/pkcs11/common/key.c
+++ b/usr/lib/pkcs11/common/key.c
@@ -3014,6 +3014,7 @@ kea_priv_set_default_attributes( TEMPLATE *tmpl, CK_ULONG mode )
    value_attr      = (CK_ATTRIBUTE *)malloc( sizeof(CK_ATTRIBUTE) );
 
    if (!type_attr || !prime_attr || !base_attr || !value_attr || !subprime_attr) {
+      if (type_attr)    free( type_attr );
       if (prime_attr)    free( prime_attr );
       if (subprime_attr) free( subprime_attr );
       if (base_attr)     free( base_attr );
diff --git a/usr/lib/pkcs11/api/api_interface.c b/usr/lib/pkcs11/api/api_interface.c
index 0a56a57..ae52d04 100755
--- a/usr/lib/pkcs11/api/api_interface.c
+++ b/usr/lib/pkcs11/api/api_interface.c
@@ -3296,6 +3296,7 @@ C_OpenSession(CK_SLOT_ID slotID,
 		}
 	} else {
 		TRACE_ERROR("%s\n", ock_err(ERR_FUNCTION_NOT_SUPPORTED));
+		free(apiSessp);
 		rv = CKR_FUNCTION_NOT_SUPPORTED;
 	}
 done:
diff --git a/usr/lib/pkcs11/common/key_mgr.c b/usr/lib/pkcs11/common/key_mgr.c
index 3b13e2c..90f73a2 100755
--- a/usr/lib/pkcs11/common/key_mgr.c
+++ b/usr/lib/pkcs11/common/key_mgr.c
@@ -1060,6 +1060,7 @@ key_mgr_wrap_key( SESSION           * sess,
    rc = encr_mgr_init( sess, ctx, OP_WRAP, mech, h_wrapping_key );
    if (rc != CKR_OK){
       TRACE_DEVEL("encr_mgr_init failed.\n");
+      free( ctx );
       return rc;
    }
    // do the encryption and clean up.  at this point, 'value' may or may not
diff --git a/usr/lib/pkcs11/common/template.c b/usr/lib/pkcs11/common/template.c
index 46488f6..9730908 100755
--- a/usr/lib/pkcs11/common/template.c
+++ b/usr/lib/pkcs11/common/template.c
@@ -982,6 +982,7 @@ template_unflatten_withSize( TEMPLATE ** new_tmpl,
    ptr = buf;
    for (i=0; i < count; i++) {
    if (buf_size >= 0 && ((ptr + sizeof(CK_ATTRIBUTE)) > (buf + buf_size))) {
+       template_free( tmpl );
        return CKR_FUNCTION_FAILED;
    }
 
@@ -998,6 +999,8 @@ template_unflatten_withSize( TEMPLATE ** new_tmpl,
 
       //if a buffer size is given, make sure it doesn't get overrun
       if (buf_size >= 0 && (((void*)a1_64 + len) > ((void*)buf + buf_size))) {
+          free( a2 );
+          template_free( tmpl );
           return CKR_FUNCTION_FAILED;
       }
       memcpy( a2, a1_64, len );
@@ -1019,6 +1022,7 @@ template_unflatten_withSize( TEMPLATE ** new_tmpl,
 
          a2 = (CK_ATTRIBUTE *)malloc(len);
          if (!a2){
+            template_free( tmpl );
             TRACE_ERROR("%s\n", ock_err(ERR_HOST_MEMORY));
             return CKR_HOST_MEMORY;
          }
@@ -1062,6 +1066,8 @@ template_unflatten_withSize( TEMPLATE ** new_tmpl,
 		    pb += sizeof(CK_ATTRIBUTE_32);
             //if a buffer size is given, make sure it doesn't get overrun
             if (buf_size >= 0 && (pb + a1->ulValueLen) > (buf + buf_size)) {
+                free( a2 );
+                template_free( tmpl );
                 return CKR_FUNCTION_FAILED;
             }
             memcpy( pb2, pb, a1->ulValueLen );
------------------------------------------------------------------------------
_______________________________________________
Opencryptoki-tech mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/opencryptoki-tech

Reply via email to