Fix probable causes for crashes in free()

While requesting for the next array of random bytes
we were cleary malloc'ing memory of the wrong size
(sizeof(length) instead of length).

This commit includes the bug fix and re-enables
SecureRandomTest.

.github/workflows/maven.yml

@@ -47,4 +47,6 @@ jobs:
       if: always()
       with:
         name: maven-surefire-reports
-        path: ${{ github.workspace }}/target/surefire-reports
+        path: |
+          ${{ github.workspace }}/target/surefire-reports
+          ${{ github.workspace }}/build/test/test.out

pom.xml

@@ -172,9 +172,6 @@
                     <includes>
                         <include>**/*Test.java</include>
                     </includes>
-                    <excludes>
-                       <exclude>**/*SecureRandomTest.java</exclude>
-                    </excludes>
                 </configuration>
             </plugin>           
         </plugins>

src/main/native/c/com_canonical_openssl_drbg_OpenSSLDrbg.c

@@ -45,23 +45,23 @@ void populate_params(DRBGParams *params, int strength, int prediction_resistance
 JNIEXPORT jlong JNICALL Java_com_canonical_openssl_drbg_OpenSSLDrbg_init
   (JNIEnv *env, jobject this, jstring name, jint strength, jboolean prediction_resistance, jboolean reseeding, jbyteArray personalization_string) {
     const char *name_string = (*env)->GetStringUTFChars(env, name, 0);
-    byte *pstr_bytes = NULL;
+    byte *ps_bytes_native = NULL;
     jsize pstr_length = 0;
 
     if (personalization_string != NULL) { 
         pstr_length = (*env)->GetArrayLength(env, personalization_string);
-        pstr_bytes = (*env)->GetByteArrayElements(env, personalization_string, NULL);
+        byte *pstr_bytes = (*env)->GetByteArrayElements(env, personalization_string, NULL);
+        ps_bytes_native = (byte *)malloc(pstr_length);
+        memcpy(ps_bytes_native, pstr_bytes, pstr_length);
+        (*env)->ReleaseByteArrayElements(env, personalization_string, pstr_bytes, JNI_ABORT);
     }
 
     DRBGParams *params = (DRBGParams *)malloc(sizeof(DRBGParams));
-    populate_params(params, strength, prediction_resistance, reseeding, pstr_bytes, pstr_length, NULL, 0);
+    populate_params(params, strength, prediction_resistance, reseeding, ps_bytes_native, pstr_length, NULL, 0);
 
     DRBG* drbg = create_DRBG_with_params(name_string, NULL, params);
     (*env)->ReleaseStringUTFChars(env, name, name_string);
 
-    if (personalization_string != NULL) {
-        (*env)->ReleaseByteArrayElements(env, personalization_string, pstr_bytes, JNI_ABORT);
-    }
     return (jlong)drbg; 
 }
 
@@ -72,31 +72,29 @@ JNIEXPORT jlong JNICALL Java_com_canonical_openssl_drbg_OpenSSLDrbg_init
  */
 JNIEXPORT void JNICALL Java_com_canonical_openssl_drbg_OpenSSLDrbg_nextBytes0
   (JNIEnv *env, jobject this, jbyteArray out_bytes, jint strength, jboolean prediction_resistance , jbyteArray additional_input) {
-
-    int additional_input_length = 0;
-    jbyte *additional_input_bytes = NULL;
-
+    byte *ai_bytes_native = NULL;
+    jsize additional_input_length = 0;
     jclass clazz = (*env)->GetObjectClass(env, this);
     jfieldID drbg_id = (*env)->GetFieldID(env, clazz, "drbgContext", "J");
     jlong drbg_handle = (*env)->GetLongField(env, this, drbg_id);
 
     int output_bytes_length = (*env)->GetArrayLength(env, out_bytes);
-    byte *output_bytes = (byte *)malloc(sizeof(output_bytes_length));
+    byte *output_bytes = (byte *)malloc(output_bytes_length);
 
     if (additional_input != NULL) {
         additional_input_length = (*env)->GetArrayLength(env, additional_input);
-        additional_input_bytes = (*env)->GetByteArrayElements(env, additional_input, NULL);
+        jbyte *additional_input_bytes = (*env)->GetByteArrayElements(env, additional_input, NULL);
+        ai_bytes_native = (byte*)malloc(additional_input_length);
+        memcpy(ai_bytes_native, additional_input_bytes, additional_input_length);
+        (*env)->ReleaseByteArrayElements(env, additional_input, additional_input_bytes, JNI_ABORT);
     }
 
     DRBGParams *params = (DRBGParams *)malloc(sizeof(DRBGParams));
-    populate_params(params, strength, prediction_resistance, 0, NULL, 0, (byte *)additional_input_bytes, additional_input_length);
+    populate_params(params, strength, prediction_resistance, 0, NULL, 0, ai_bytes_native, additional_input_length);
 
     next_rand_with_params((DRBG *)drbg_handle, output_bytes, output_bytes_length, params);
 
     (*env)->SetByteArrayRegion(env, out_bytes, 0, output_bytes_length, output_bytes);
-    if (additional_input != NULL) {
-        (*env)->ReleaseByteArrayElements(env, additional_input, additional_input_bytes, JNI_ABORT);
-    }
 }
 
 /*
@@ -139,6 +137,7 @@ JNIEXPORT void JNICALL Java_com_canonical_openssl_drbg_OpenSSLDrbg_reseed0
     }
 }
 
+#define MAX_SEED_BYTES 256
 /*
  * Class:     com_canonical_openssl_OpenSSLDrbg
  * Method:    generateSeed0
@@ -151,18 +150,17 @@ JNIEXPORT jbyteArray JNICALL Java_com_canonical_openssl_drbg_OpenSSLDrbg_generat
     jfieldID drbg_id = (*env)->GetFieldID(env, clazz, "drbgContext", "J");
     jlong drbg_handle = (*env)->GetLongField(env, this, drbg_id);
 
-    byte *output = (byte *)malloc(num_bytes);
-
-    if (output == NULL) {
-        return NULL;
+    if (num_bytes > 256) {
+        num_bytes = 256;
     }
 
+    byte output[MAX_SEED_BYTES];
+
     generate_seed((DRBG*)drbg_handle, output, num_bytes);
 
     jbyteArray ret_array = (*env)->NewByteArray(env, num_bytes);
-    (*env)->SetBooleanArrayRegion(env, ret_array, 0, num_bytes, output);
+    (*env)->SetByteArrayRegion(env, ret_array, 0, num_bytes, output);
 
-    free(output);
     return ret_array;
 }
 

src/main/native/c/drbg.c

@@ -16,6 +16,7 @@
  */
 #include <drbg.h>
 #include <stdio.h>
+#include <sys/random.h>
 #include <unistd.h>
 
 DRBGParams NO_PARAMS = { DEFAULT_STRENGTH, 0, 0, NULL, 0, NULL, 0 };
@@ -57,7 +58,7 @@ DRBG* create_DRBG(const char* name, DRBG* parent) {
 DRBG* create_DRBG_with_params(const char* name, DRBG* parent, DRBGParams *drbg_params) {
     EVP_RAND *rand = EVP_RAND_fetch(NULL, name, NULL);
     if (NULL == rand) {
-        fprintf(stderr, "Couldn't allocate EVP_RAND\n");
+        fprintf(stderr, "Couldn't allocate EVP_RAND: %s\n", name);
         return NULL;
     }
 
@@ -93,6 +94,9 @@ DRBG* create_DRBG_with_params(const char* name, DRBG* parent, DRBGParams *drbg_p
 }
 
 int free_DRBGParams(DRBGParams *params) {
+    if (params == NULL) {
+        return 0;
+    }
     FREE_IF_NON_NULL(params->additional_data);
     FREE_IF_NON_NULL(params->personalization_str);
     FREE_IF_NON_NULL(params);
@@ -103,23 +107,10 @@ int free_DRBG(DRBG *generator) {
     if (generator == NULL) {
         return 0;
     }
-
+    free_DRBGParams(generator->params);
+    free_DRBG(generator->parent);
     FREE_IF_NON_NULL(generator->seed);
-    if (generator->context != NULL) {
-        EVP_RAND_CTX_free(generator->context);
-        generator->context = NULL;
-    }
-
-    if (generator->params != NULL) {
-        free_DRBGParams(generator->params);
-        generator->params = NULL;
-    }
-
-    if (generator->parent != NULL) {
-        free_DRBG(generator->parent);
-        generator->parent = NULL;
-    }
-
+    EVP_RAND_CTX_free(generator->context);
     free(generator);
     return 1;
 }
@@ -157,7 +148,7 @@ int generate_seed(DRBG* generator, byte output[], int n_bytes) {
     if (parent != NULL) {
         return next_rand(parent, output, n_bytes);
     } else {
-        return getentropy(output, n_bytes);
+        return getrandom(output, n_bytes, 0);
     }
 }
 
@@ -168,7 +159,7 @@ void reseed(DRBG* generator) {
 void reseed_with_params(DRBG *generator, DRBGParams *params) {
     byte seed[128]; // TODO: what should the default seed size be?
     size_t length = 128;
-    getentropy(seed, length);
+    getrandom(seed, length, 0);
     EVP_RAND_reseed(generator->context, params->prediction_resistance, seed, length, params->additional_data, params->additional_data_length);
 }
 

src/main/native/c/signature.c

@@ -16,6 +16,7 @@
  */
 #include "signature.h"
 #include "jssl.h"
+#include <openssl/err.h>
 #include <openssl/rsa.h>
 #include <openssl/core_names.h>
 

src/test/java/SecureRandomTest.java

@@ -169,8 +169,8 @@ public void testDRBGCreationNextBytesWithParams() throws NoSuchAlgorithmExceptio
         testArrayInequality(hash1, hash2);
     }
 
-    /* disabled - needs a "double-free" investigation" */
-    private void testDRBGCreationWithParamsNextBytesWithParams() throws NoSuchAlgorithmException, NoSuchProviderException {
+    @Test
+    public void testDRBGCreationWithParamsNextBytesWithParams() throws NoSuchAlgorithmException, NoSuchProviderException {
         SecureRandomParameters params = DrbgParameters.instantiation(128, Capability.PR_AND_RESEED, "FIPSPROTOTYPE".getBytes());
         SecureRandomParameters nbParams = DrbgParameters.nextBytes(128, true, "ADDITIONALINPUT".getBytes());
 

src/test/runner.py

@@ -34,7 +34,7 @@
 }
 
 def run_native_test(name):
-  return os.system(f"build/test/bin/{name} > /dev/null 2>&1")
+  return os.system(f"build/test/bin/{name} >> build/test/test.out 2>&1") 
 
 for test in tests.keys():
   name = tests[test]