Fix memory leak computing ECDH secrets

The context allocated in the method `XECKEY.computeECDHSecret` was never freed when a key was successfully generated. This update frees memory associated with the context prior to return of the secret key bytes. Whitespace and formatting was also done to make use of brackets for if statements. Fixes IBM#387 Signed-off-by: Jason Katonica <[email protected]>
jasonkatonica · Dec 11, 2024 · c226fef · c226fef
1 parent 42d46ce
commit c226fef
Showing 1 changed file with 36 additions and 18 deletions.
diff --git a/src/main/native/ECKey.c b/src/main/native/ECKey.c
@@ -2121,12 +2121,14 @@ JNIEXPORT jbyteArray JNICALL Java_com_ibm_crypto_plus_provider_ock_NativeInterfa
   jbyteArray           retSecretBytes = NULL;
   size_t               secret_key_len = 0;
 
-
-  if( debug ) gslogFunctionEntry(functionName);
+  if( debug ) {
+    gslogFunctionEntry(functionName);
+  }
 
   gen_ctx = ICC_EVP_PKEY_CTX_new(ockCtx,(ICC_EVP_PKEY *) ockPrivXecKey,NULL); /* Set private key */
-  if(gen_ctx == NULL) throwOCKException(env, 0, "NULL from ICC_EVP_PKEY_CTX_new"); 
-  else {
+  if ( NULL == gen_ctx ) {
+     throwOCKException(env, 0, "NULL from ICC_EVP_PKEY_CTX_new");
+  } else {
     ICC_EVP_PKEY_derive_init(ockCtx, gen_ctx);
     ICC_EVP_PKEY_derive_set_peer(ockCtx, gen_ctx, ockPubXecKey); /* Set public key */
     if (secretBufferSize > 0) {
@@ -2135,28 +2137,44 @@ JNIEXPORT jbyteArray JNICALL Java_com_ibm_crypto_plus_provider_ock_NativeInterfa
         ICC_EVP_PKEY_derive(ockCtx, gen_ctx, NULL, &secret_key_len); /* Get secret key size */
     }
     secretBytes = (*env)->NewByteArray(env, secret_key_len); /* Create Java secret bytes array with size */
-    if( secretBytes == NULL ) throwOCKException(env, 0, "NewByteArray failed"); 
-    else {
+    if( NULL == secretBytes ) {
+      throwOCKException(env, 0, "NewByteArray failed"); 
+    } else {
       secretBytesNative = (unsigned char*)((*env)->GetPrimitiveArrayCritical(env, secretBytes, &isCopy));
-      if( secretBytesNative == NULL ) throwOCKException(env, 0, "NULL from GetPrimitiveArrayCritical"); 
-      else {
+      if( NULL == secretBytesNative ) {
+        throwOCKException(env, 0, "NULL from GetPrimitiveArrayCritical");
+      } else {
         ICC_EVP_PKEY_derive(ockCtx, gen_ctx, secretBytesNative, &secret_key_len);
         retSecretBytes = secretBytes;
-        if( secretBytesNative != NULL ) (*env)->ReleasePrimitiveArrayCritical(env, secretBytes, secretBytesNative, 0);
-        if((secretBytes != NULL) && (retSecretBytes == NULL)) (*env)->DeleteLocalRef(env, secretBytes);
-        if( debug ) gslogFunctionExit(functionName);
+        ICC_EVP_PKEY_CTX_free(ockCtx, gen_ctx);
+        (*env)->ReleasePrimitiveArrayCritical(env, secretBytes, secretBytesNative, 0);
+        if( NULL == retSecretBytes ) {
+          (*env)->DeleteLocalRef(env, secretBytes);
+        }
+        if( debug ) {
+          gslogFunctionExit(functionName);
+        }
         return retSecretBytes;
       }
     }
-    if (gen_ctx != NULL) {
-      ICC_EVP_PKEY_CTX_free(ockCtx,gen_ctx);
-      gen_ctx = NULL;
-    }
   }
 
-  if( secretBytesNative != NULL ) (*env)->ReleasePrimitiveArrayCritical(env, secretBytes, secretBytesNative, 0);
-  if((secretBytes != NULL) && (retSecretBytes == NULL)) (*env)->DeleteLocalRef(env, secretBytes);
-  if( debug ) gslogFunctionExit(functionName);
+  if (NULL != gen_ctx) {
+    ICC_EVP_PKEY_CTX_free(ockCtx, gen_ctx);
+  }
+
+  if( NULL != secretBytesNative ) {
+    (*env)->ReleasePrimitiveArrayCritical(env, secretBytes, secretBytesNative, 0);
+  }
+
+  if( ( NULL != secretBytes ) && ( NULL == retSecretBytes ) ) {
+    (*env)->DeleteLocalRef(env, secretBytes);
+  }
+
+  if( debug ) {
+    gslogFunctionExit(functionName);
+  }
+
   return NULL;
 }