Fix several leaks in OpenSSL JNI cipher suites code.

I first spotted the missing ReleaseStringUTFChars, but then noticed all
the duplication in the cipher suites functions, and noticed that no
copy appeared to be completely correct. The factored-out replacements
shouldn't leak, and should check all error conditions.

3 files changed, 69 insertions, 146 deletions

diff --git a/x-net/src/main/native/org_apache_harmony_xnet_provider_jsse_OpenSSLServerSocketImpl.cpp b/x-net/src/main/native/org_apache_harmony_xnet_provider_jsse_OpenSSLServerSocketImpl.cpp
index 2a55bbc..eec2efe 100644
--- a/x-net/src/main/native/org_apache_harmony_xnet_provider_jsse_OpenSSLServerSocketImpl.cpp
+++ b/x-net/src/main/native/org_apache_harmony_xnet_provider_jsse_OpenSSLServerSocketImpl.cpp
@@ -157,38 +157,13 @@ static void org_apache_harmony_xnet_provider_jsse_OpenSSLServerSocketImpl_setena
 static jobjectArray org_apache_harmony_xnet_provider_jsse_OpenSSLServerSocketImpl_getsupportedciphersuites(JNIEnv* env,
         jobject object)
 {
-    SSL_CTX* ctx;
-    SSL* ssl;
-    STACK_OF(SSL_CIPHER) *sk;
-    jobjectArray ret;
-    int i;
-    const char *c;
-
-    ctx = SSL_CTX_new(SSLv23_server_method());
-    ssl = SSL_new(ctx);
-    sk=SSL_get_ciphers(ssl);
-
-    ret= (jobjectArray)env->NewObjectArray(5,
-         env->FindClass("java/lang/String"),
-         env->NewStringUTF(""));
-
-    i = 0;
-    while (SSL_get_cipher_list(ssl,i) != NULL) {
-        i++;
+    SSL_CTX* ssl_ctx = SSL_CTX_new(SSLv23_server_method());
+    if (ssl_ctx == NULL) {
+        return NULL;
     }
-
-    ret = (jobjectArray)env->NewObjectArray(i,
-        env->FindClass("java/lang/String"),
-        env->NewStringUTF(""));
-
-    for (i=0; ; i++) {
-        c=SSL_get_cipher_list(ssl,i);
-        if (c == NULL) break;
-
-        env->SetObjectArrayElement(ret,i,env->NewStringUTF(c));
-    }
-
-    return ret;
+    jobjectArray result = makeCipherList(env, ssl_ctx);
+    SSL_CTX_free(ssl_ctx);
+    return result;
 }
 
 /**
@@ -198,52 +173,20 @@ static jobjectArray org_apache_harmony_xnet_provider_jsse_OpenSSLServerSocketImp
 static jobjectArray org_apache_harmony_xnet_provider_jsse_OpenSSLServerSocketImpl_getenabledciphersuites(JNIEnv* env,
         jobject object)
 {
-    SSL_CTX* ctx;
-    SSL* ssl;
-    jobjectArray ret;
-    int i;
-    const char *c;
-
-    ctx = (SSL_CTX*)env->GetIntField(object, field_ssl_ctx);
-    ssl = SSL_new(ctx);
-
-    i = 0;
-    while (SSL_get_cipher_list(ssl,i) != NULL) {
-        i++;
-    }
-
-    ret = (jobjectArray)env->NewObjectArray(i,
-        env->FindClass("java/lang/String"),
-        env->NewStringUTF(""));
-
-    for (i = 0; ; i++) {
-        c = SSL_get_cipher_list(ssl,i);
-        if (c == NULL) break;
-
-        env->SetObjectArrayElement(ret,i,env->NewStringUTF(c));
-    }
-
-    return ret;
+    SSL_CTX* ssl_ctx =
+            reinterpret_cast<SSL_CTX*>(env->GetIntField(object, field_ssl_ctx));
+    return makeCipherList(env, ssl_ctx);
 }
 
 /**
  * Sets the ciphers suites that are enabled in the OpenSSL server.
  */
 static void org_apache_harmony_xnet_provider_jsse_OpenSSLServerSocketImpl_setenabledciphersuites(JNIEnv* env,
-        jobject object, jstring controlstring)
+        jobject object, jstring controlString)
 {
-    SSL_CTX* ctx;
-    const char *str;
-    int ret;
-
-    ctx = (SSL_CTX*)env->GetIntField(object, field_ssl_ctx);
-    str = env->GetStringUTFChars(controlstring, 0);
-    ret = SSL_CTX_set_cipher_list(ctx, str);
-
-    if(ret == 0) {
-        jniThrowException(env, "java/lang/IllegalArgumentException",
-                          "Illegal cipher suite strings.");
-    }    
+    SSL_CTX* ssl_ctx =
+            reinterpret_cast<SSL_CTX*>(env->GetIntField(object, field_ssl_ctx));
+    setEnabledCipherSuites(env, controlString, ssl_ctx);
 }
 
 /**
diff --git a/x-net/src/main/native/org_apache_harmony_xnet_provider_jsse_OpenSSLSocketImpl.cpp b/x-net/src/main/native/org_apache_harmony_xnet_provider_jsse_OpenSSLSocketImpl.cpp
index 8f046ce..88549e1 100644
--- a/x-net/src/main/native/org_apache_harmony_xnet_provider_jsse_OpenSSLSocketImpl.cpp
+++ b/x-net/src/main/native/org_apache_harmony_xnet_provider_jsse_OpenSSLSocketImpl.cpp
@@ -1414,45 +1414,13 @@ static void org_apache_harmony_xnet_provider_jsse_OpenSSLSocketImpl_setenabledpr
 static jobjectArray org_apache_harmony_xnet_provider_jsse_OpenSSLSocketImpl_getsupportedciphersuites(JNIEnv* env,
         jobject object)
 {
-    SSL_CTX* ssl_ctx;
-    SSL* ssl;
-    jobjectArray ret;
-    int i;
-    const char *c;
-
-    ssl_ctx = SSL_CTX_new(SSLv23_client_method());
-
+    SSL_CTX* ssl_ctx = SSL_CTX_new(SSLv23_client_method());
     if (ssl_ctx == NULL) {
         return NULL;
     }
-
-    ssl = SSL_new(ssl_ctx);
-
-    if (ssl == NULL) {
-        SSL_CTX_free(ssl_ctx);
-        return NULL;
-    }
-    
-    i = 0;
-    while (SSL_get_cipher_list(ssl,i) != NULL) {
-        i++;
-    }
-
-    ret = (jobjectArray)env->NewObjectArray(i,
-        env->FindClass("java/lang/String"),
-        env->NewStringUTF(""));
-
-    for (i=0; ; i++) {
-        c=SSL_get_cipher_list(ssl,i);
-        if (c == NULL) break;
-
-        env->SetObjectArrayElement(ret,i,env->NewStringUTF(c));
-    }
-
-    SSL_free(ssl);
+    jobjectArray result = makeCipherList(env, ssl_ctx);
     SSL_CTX_free(ssl_ctx);
-
-    return ret;
+    return result;
 }
 
 /**
@@ -1462,57 +1430,66 @@ static jobjectArray org_apache_harmony_xnet_provider_jsse_OpenSSLSocketImpl_gets
 static jobjectArray org_apache_harmony_xnet_provider_jsse_OpenSSLSocketImpl_getenabledciphersuites(JNIEnv* env,
         jobject object)
 {
-    SSL_CTX* ssl_ctx;
-    SSL* ssl;
-    jobjectArray ret;
-    int i;
-    const char *c;
-
-    ssl = getSslPointer(env, object, false);
-    if (ssl == NULL) {
-        ssl_ctx = (SSL_CTX*)env->GetIntField(object, field_ssl_ctx);
-        ssl = SSL_new(ssl_ctx);
-        env->SetIntField(object, field_ssl, (int)ssl);
-    }
-
-    i = 0;
-    while (SSL_get_cipher_list(ssl,i) != NULL) {
-        i++;
-    }
-
-    ret = (jobjectArray)env->NewObjectArray(i,
-        env->FindClass("java/lang/String"),
-        env->NewStringUTF(""));
-
-    for (i = 0; ; i++) {
-        c = SSL_get_cipher_list(ssl,i);
-        if (c == NULL) break;
-
-        env->SetObjectArrayElement(ret,i,env->NewStringUTF(c));
-    }
-
-    return ret;
+    SSL_CTX* ssl_ctx =
+            reinterpret_cast<SSL_CTX*>(env->GetIntField(object, field_ssl_ctx));
+    return makeCipherList(env, ssl_ctx);
 }
 
 /**
  * Sets the ciphers suites that are enabled in the OpenSSL client.
  */
 static void org_apache_harmony_xnet_provider_jsse_OpenSSLSocketImpl_setenabledciphersuites(JNIEnv* env, jobject object,
-        jstring controlstring)
+        jstring controlString)
 {
-    SSL_CTX* ctx;
-    const char *str;
-    int ret;
+    SSL_CTX* ssl_ctx =
+            reinterpret_cast<SSL_CTX*>(env->GetIntField(object, field_ssl_ctx));
+    setEnabledCipherSuites(env, controlString, ssl_ctx);
+}
 
-    ctx = (SSL_CTX*)env->GetIntField(object, field_ssl_ctx);
-    str = env->GetStringUTFChars(controlstring, 0);
-    ret = SSL_CTX_set_cipher_list(ctx, str);
+static jobjectArray makeCipherList(JNIEnv* env, SSL* ssl) {
+    // Count the ciphers.
+    int cipherCount = 0;
+    while (SSL_get_cipher_list(ssl, cipherCount) != NULL) {
+        ++cipherCount;
+    }
 
-    if (ret == 0) {
+    // Create a String[].
+    jclass stringClass = env->FindClass("java/lang/String");
+    if (stringClass == NULL) {
+        return NULL;
+    }
+    jobjectArray array = env->NewObjectArray(cipherCount, stringClass, NULL);
+    if (array == NULL) {
+        return NULL;
+    }
+
+    // Fill in the cipher names.
+    for (int i = 0; i < cipherCount; ++i) {
+        const char* c = SSL_get_cipher_list(ssl, i);
+        env->SetObjectArrayElement(array, i, env->NewStringUTF(c));
+    }
+    return array;
+}
+
+jobjectArray makeCipherList(JNIEnv* env, SSL_CTX* ssl_ctx) {
+    SSL* ssl = SSL_new(ssl_ctx);
+    if (ssl == NULL) {
+        return NULL;
+    }
+    jobjectArray result = makeCipherList(env, ssl);
+    SSL_free(ssl);
+    return result;
+}
+
+void setEnabledCipherSuites(JNIEnv* env, jstring controlString, SSL_CTX* ssl_ctx) {
+    const char* str = env->GetStringUTFChars(controlString, NULL);
+    int rc = SSL_CTX_set_cipher_list(ssl_ctx, str);
+    env->ReleaseStringUTFChars(controlString, str);
+    if (rc == 0) {
         freeSslErrorState();
         jniThrowException(env, "java/lang/IllegalArgumentException",
                           "Illegal cipher suite strings.");
-    }    
+    }
 }
 
 #define SSL_AUTH_MASK           0x00007F00L
diff --git a/x-net/src/main/native/org_apache_harmony_xnet_provider_jsse_common.h b/x-net/src/main/native/org_apache_harmony_xnet_provider_jsse_common.h
index 409b4ec..e78cdd8 100644
--- a/x-net/src/main/native/org_apache_harmony_xnet_provider_jsse_common.h
+++ b/x-net/src/main/native/org_apache_harmony_xnet_provider_jsse_common.h
@@ -26,10 +26,10 @@
 /**
  * Structure to hold together useful JNI variables.
  */
-typedef struct {
+struct mydata_t {
     JNIEnv* env;
     jobject object;
-} mydata_t;
+};
 
 /**
  * Gives an array back containing all the X509 certificate's bytes.
@@ -117,4 +117,7 @@ static int verify_callback(int preverify_ok, X509_STORE_CTX *x509_store_ctx)
     return 1;
 }
 
+extern jobjectArray makeCipherList(JNIEnv* env, SSL_CTX* ssl);
+extern void setEnabledCipherSuites(JNIEnv* env, jstring controlString, SSL_CTX* ssl_ctx);
+
 #endif