Delete local references to Java-returned result arrays in JNI callbacks (#1016)

Motivation:

resultArray in cert_compress.c compress()/decompress() and resultBytes
in the SSL_PRIVATE_KEY_METHOD callbacks in sslcontext.c are obtained
from CallObjectMethod/GetObjectField but were never deleted on any
exit path. These are invoked repeatedly as native callbacks from
OpenSSL/BoringSSL (once per certificate needing (de)compression, or
per sign/decrypt/complete call), so the leaked local references
accumulate and can exhaust the JNI local reference table.

Modifications:

- cert_compress.c: delete resultArray before every return in compress()
  and decompress() once it has been obtained.
- sslcontext.c: delete resultBytes at the complete: label in
  tcn_private_key_sign_java and tcn_private_key_decrypt_java, and
  before every return in tcn_private_key_complete_java.

Result:

No longer leak local references to compression/private-key callback
result arrays.

Author: normanmaurer

openssl-dynamic/src/main/c/cert_compress.c

@@ -53,14 +53,17 @@ static int compress(jobject compression_algorithm, jmethodID compress_method, SS
     int resultLen = (*e)->GetArrayLength(e, resultArray);
     uint8_t* outData = NULL;
     if (!CBB_reserve(out, &outData, resultLen)) {
+        NETTY_JNI_UTIL_DELETE_LOCAL(e, resultArray);
         return 0; // Unable to reserve space for compressed data
     }
     jbyte* resultData = (*e)->GetByteArrayElements(e, resultArray, NULL);
     if (resultData == NULL) {
+        NETTY_JNI_UTIL_DELETE_LOCAL(e, resultArray);
         return 0;
     }
     memcpy(outData, resultData, resultLen);
     (*e)->ReleaseByteArrayElements(e, resultArray, resultData, JNI_ABORT);
+    NETTY_JNI_UTIL_DELETE_LOCAL(e, resultArray);
     if (!CBB_did_write(out, resultLen)) {
         return 0; // Unable to advance bytes written to CBB
     }
@@ -102,20 +105,24 @@ static int decompress(jobject compression_algorithm, jmethodID decompress_method
 
     int resultLen = (*e)->GetArrayLength(e, resultArray);
     if (uncompressed_len != resultLen) {
+        NETTY_JNI_UTIL_DELETE_LOCAL(e, resultArray);
         return 0; // Unexpected uncompressed length
     }
     jbyte* resultData = (*e)->GetByteArrayElements(e, resultArray, NULL);
     if (resultData == NULL) {
+        NETTY_JNI_UTIL_DELETE_LOCAL(e, resultArray);
         return 0;
     }
     uint8_t* outData;
     if (!((*out) = CRYPTO_BUFFER_alloc(&outData, uncompressed_len))) {
         // Unable to allocate certificate decompression buffer
         (*e)->ReleaseByteArrayElements(e, resultArray, resultData, JNI_ABORT);
+        NETTY_JNI_UTIL_DELETE_LOCAL(e, resultArray);
         return 0;
     }
     memcpy(outData, resultData, uncompressed_len);
     (*e)->ReleaseByteArrayElements(e, resultArray, resultData, JNI_ABORT);
+    NETTY_JNI_UTIL_DELETE_LOCAL(e, resultArray);
     return 1; // Success
 }
 

openssl-dynamic/src/main/c/sslcontext.c

@@ -2281,6 +2281,7 @@ static enum ssl_private_key_result_t tcn_private_key_sign_java(SSL *ssl, uint8_t
 complete:
     // Free up any allocated memory and return.
     NETTY_JNI_UTIL_DELETE_LOCAL(e, inputArray);
+    NETTY_JNI_UTIL_DELETE_LOCAL(e, resultBytes);
     NETTY_JNI_UTIL_DELETE_LOCAL(e, sslPrivateKeyMethodSignTask_class);
 
     return ret;
@@ -2350,6 +2351,7 @@ static enum ssl_private_key_result_t tcn_private_key_decrypt_java(SSL *ssl, uint
 complete:
     // Delete the local reference as this is executed by a callback.
     NETTY_JNI_UTIL_DELETE_LOCAL(e, inArray);
+    NETTY_JNI_UTIL_DELETE_LOCAL(e, resultBytes);
     NETTY_JNI_UTIL_DELETE_LOCAL(e, sslPrivateKeyMethodDecryptTask_class);
     return ret;
 }
@@ -2388,21 +2390,25 @@ static enum ssl_private_key_result_t tcn_private_key_complete_java(SSL *ssl, uin
         state->ssl_task = NULL;
 
         if (returnValue != 1 || resultBytes == NULL) {
+            NETTY_JNI_UTIL_DELETE_LOCAL(e, resultBytes);
             return ssl_private_key_failure;
         }
 
         arrayLen = (*e)->GetArrayLength(e, resultBytes);
         if (max_out < arrayLen) {
              // We need to fail as otherwise we would end up writing into memory which does not
              // belong to us.
+            NETTY_JNI_UTIL_DELETE_LOCAL(e, resultBytes);
             return ssl_private_key_failure;
         }
         if ((b = (*e)->GetByteArrayElements(e, resultBytes, NULL)) == NULL) {
+            NETTY_JNI_UTIL_DELETE_LOCAL(e, resultBytes);
             return ssl_private_key_failure;
         }
         memcpy(out, b, arrayLen);
         (*e)->ReleaseByteArrayElements(e, resultBytes, b, JNI_ABORT);
         *out_len = arrayLen;
+        NETTY_JNI_UTIL_DELETE_LOCAL(e, resultBytes);
         return ssl_private_key_success;
     }
     return ssl_private_key_failure;