Check appropriate OSSL_PARAM_get_* functions for NULL

The base type OSSL_PARAM getters will NULL deref if they are initalized
as null.  Add NULL checks for those parameters that have no expectation
of returning null (int32/64/uint32/64/BN).  Other types can be left as
allowing NULL, as a NULL setting may be meaningful (string, utf8str,
octet string, etc).

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/23083)

crypto/params.c

@@ -197,6 +197,10 @@ static int unsigned_from_unsigned(void *dest, size_t dest_len,
 /* General purpose get integer parameter call that handles odd sizes */
 static int general_get_int(const OSSL_PARAM *p, void *val, size_t val_size)
 {
+    if (p->data == NULL) {
+        err_null_argument;
+        return 0;
+    }
     if (p->data_type == OSSL_PARAM_INTEGER)
         return signed_from_signed(val, val_size, p->data, p->data_size);
     if (p->data_type == OSSL_PARAM_UNSIGNED_INTEGER)
@@ -226,6 +230,11 @@ static int general_set_int(OSSL_PARAM *p, void *val, size_t val_size)
 /* General purpose get unsigned integer parameter call that handles odd sizes */
 static int general_get_uint(const OSSL_PARAM *p, void *val, size_t val_size)
 {
+
+    if (p->data == NULL) {
+        err_null_argument;
+        return 0;
+    }
     if (p->data_type == OSSL_PARAM_INTEGER)
         return unsigned_from_signed(val, val_size, p->data, p->data_size);
     if (p->data_type == OSSL_PARAM_UNSIGNED_INTEGER)
@@ -385,6 +394,11 @@ int OSSL_PARAM_get_int32(const OSSL_PARAM *p, int32_t *val)
         return 0;
     }
 
+    if (p->data == NULL) {
+        err_null_argument;
+        return 0;
+    }
+
     if (p->data_type == OSSL_PARAM_INTEGER) {
 #ifndef OPENSSL_SMALL_FOOTPRINT
         int64_t i64;
@@ -534,6 +548,11 @@ int OSSL_PARAM_get_uint32(const OSSL_PARAM *p, uint32_t *val)
         return 0;
     }
 
+    if (p->data == NULL) {
+        err_null_argument;
+        return 0;
+    }
+
     if (p->data_type == OSSL_PARAM_UNSIGNED_INTEGER) {
 #ifndef OPENSSL_SMALL_FOOTPRINT
         uint64_t u64;
@@ -685,6 +704,11 @@ int OSSL_PARAM_get_int64(const OSSL_PARAM *p, int64_t *val)
         return 0;
     }
 
+    if (p->data == NULL) {
+        err_null_argument;
+        return 0;
+    }
+
     if (p->data_type == OSSL_PARAM_INTEGER) {
 #ifndef OPENSSL_SMALL_FOOTPRINT
         switch (p->data_size) {
@@ -829,6 +853,11 @@ int OSSL_PARAM_get_uint64(const OSSL_PARAM *p, uint64_t *val)
         return 0;
     }
 
+    if (p->data == NULL) {
+        err_null_argument;
+        return 0;
+    }
+
     if (p->data_type == OSSL_PARAM_UNSIGNED_INTEGER) {
 #ifndef OPENSSL_SMALL_FOOTPRINT
         switch (p->data_size) {
@@ -1040,7 +1069,7 @@ int OSSL_PARAM_get_BN(const OSSL_PARAM *p, BIGNUM **val)
 {
     BIGNUM *b = NULL;
 
-    if (val == NULL || p == NULL) {
+    if (val == NULL || p == NULL || p->data == NULL) {
         err_null_argument;
         return 0;
     }
@@ -1132,7 +1161,7 @@ int OSSL_PARAM_get_double(const OSSL_PARAM *p, double *val)
     int64_t i64;
     uint64_t u64;
 
-    if (val == NULL || p == NULL) {
+    if (val == NULL || p == NULL || p->data == NULL) {
         err_null_argument;
         return 0;
     }

test/params_api_test.c

@@ -70,6 +70,49 @@ static const struct {
             0x89, 0x67, 0xf2, 0x68, 0x33, 0xa0, 0x14, 0xb0 } },
 };
 
+static int test_param_type_null(OSSL_PARAM *param)
+{
+    int rc = 0;
+    uint64_t intval;
+    double dval;
+    BIGNUM *bn;
+
+    switch(param->data_type) {
+    case OSSL_PARAM_INTEGER:
+        if (param->data_size == sizeof(int32_t))
+            rc = OSSL_PARAM_get_int32(param, (int32_t *)&intval);
+        else if (param->data_size == sizeof(uint64_t))
+            rc = OSSL_PARAM_get_int64(param, (int64_t *)&intval);
+        else
+            return 1;
+        break;
+    case OSSL_PARAM_UNSIGNED_INTEGER:
+        if (param->data_size == sizeof(uint32_t))
+            rc = OSSL_PARAM_get_uint32(param, (uint32_t *)&intval);
+        else if (param->data_size == sizeof(uint64_t))
+            rc = OSSL_PARAM_get_uint64(param, &intval);
+        else
+            rc = OSSL_PARAM_get_BN(param, &bn);
+        break;
+    case OSSL_PARAM_REAL:
+        rc = OSSL_PARAM_get_double(param, &dval);
+        break;
+    case OSSL_PARAM_UTF8_STRING:
+    case OSSL_PARAM_OCTET_STRING:
+    case OSSL_PARAM_UTF8_PTR:
+    case OSSL_PARAM_OCTET_PTR:
+        /* these are allowed to be null */
+        return 1;
+        break;
+    }
+
+    /*
+     * we expect the various OSSL_PARAM_get functions above
+     * to return failure when the data is set to NULL
+     */
+    return rc == 0;
+}
+
 static int test_param_type_extra(OSSL_PARAM *param, const unsigned char *cmp,
                                  size_t width)
 {
@@ -157,6 +200,9 @@ static int test_param_int(int n)
                        sizeof(int) : raw_values[n].len;
     OSSL_PARAM param = OSSL_PARAM_int("a", NULL);
 
+    if (!TEST_int_eq(test_param_type_null(&param), 1))
+        return 0;
+
     memset(buf, 0, sizeof(buf));
     le_copy(buf, sizeof(in), raw_values[n].value, sizeof(in));
     memcpy(&in, buf, sizeof(in));
@@ -184,6 +230,9 @@ static int test_param_long(int n)
                        ? sizeof(long int) : raw_values[n].len;
     OSSL_PARAM param = OSSL_PARAM_long("a", NULL);
 
+    if (!TEST_int_eq(test_param_type_null(&param), 1))
+        return 0;
+
     memset(buf, 0, sizeof(buf));
     le_copy(buf, sizeof(in), raw_values[n].value, sizeof(in));
     memcpy(&in, buf, sizeof(in));
@@ -210,6 +259,9 @@ static int test_param_uint(int n)
     const size_t len = raw_values[n].len >= sizeof(unsigned int) ? sizeof(unsigned int) : raw_values[n].len;
     OSSL_PARAM param = OSSL_PARAM_uint("a", NULL);
 
+    if (!TEST_int_eq(test_param_type_null(&param), 1))
+        return 0;
+
     memset(buf, 0, sizeof(buf));
     le_copy(buf, sizeof(in), raw_values[n].value, sizeof(in));
     memcpy(&in, buf, sizeof(in));
@@ -237,6 +289,9 @@ static int test_param_ulong(int n)
                        ? sizeof(unsigned long int) : raw_values[n].len;
     OSSL_PARAM param = OSSL_PARAM_ulong("a", NULL);
 
+    if (!TEST_int_eq(test_param_type_null(&param), 1))
+        return 0;
+
     memset(buf, 0, sizeof(buf));
     le_copy(buf, sizeof(in), raw_values[n].value, sizeof(in));
     memcpy(&in, buf, sizeof(in));
@@ -264,6 +319,9 @@ static int test_param_int32(int n)
                        ? sizeof(int32_t) : raw_values[n].len;
     OSSL_PARAM param = OSSL_PARAM_int32("a", NULL);
 
+    if (!TEST_int_eq(test_param_type_null(&param), 1))
+        return 0;
+
     memset(buf, 0, sizeof(buf));
     le_copy(buf, sizeof(in), raw_values[n].value, sizeof(in));
     memcpy(&in, buf, sizeof(in));
@@ -291,6 +349,9 @@ static int test_param_uint32(int n)
                        ? sizeof(uint32_t) : raw_values[n].len;
     OSSL_PARAM param = OSSL_PARAM_uint32("a", NULL);
 
+    if (!TEST_int_eq(test_param_type_null(&param), 1))
+        return 0;
+
     memset(buf, 0, sizeof(buf));
     le_copy(buf, sizeof(in), raw_values[n].value, sizeof(in));
     memcpy(&in, buf, sizeof(in));
@@ -318,6 +379,9 @@ static int test_param_int64(int n)
                        ? sizeof(int64_t) : raw_values[n].len;
     OSSL_PARAM param = OSSL_PARAM_int64("a", NULL);
 
+    if (!TEST_int_eq(test_param_type_null(&param), 1))
+        return 0;
+
     memset(buf, 0, sizeof(buf));
     le_copy(buf, sizeof(in), raw_values[n].value, sizeof(in));
     memcpy(&in, buf, sizeof(in));
@@ -345,6 +409,9 @@ static int test_param_uint64(int n)
                        ? sizeof(uint64_t) : raw_values[n].len;
     OSSL_PARAM param = OSSL_PARAM_uint64("a", NULL);
 
+    if (!TEST_int_eq(test_param_type_null(&param), 1))
+        return 0;
+
     memset(buf, 0, sizeof(buf));
     le_copy(buf, sizeof(in), raw_values[n].value, sizeof(in));
     memcpy(&in, buf, sizeof(in));
@@ -372,6 +439,9 @@ static int test_param_size_t(int n)
                        ? sizeof(size_t) : raw_values[n].len;
     OSSL_PARAM param = OSSL_PARAM_size_t("a", NULL);
 
+    if (!TEST_int_eq(test_param_type_null(&param), 1))
+        return 0;
+
     memset(buf, 0, sizeof(buf));
     le_copy(buf, sizeof(in), raw_values[n].value, sizeof(in));
     memcpy(&in, buf, sizeof(in));
@@ -399,6 +469,9 @@ static int test_param_time_t(int n)
                        ? sizeof(time_t) : raw_values[n].len;
     OSSL_PARAM param = OSSL_PARAM_time_t("a", NULL);
 
+    if (!TEST_int_eq(test_param_type_null(&param), 1))
+        return 0;
+
     memset(buf, 0, sizeof(buf));
     le_copy(buf, sizeof(in), raw_values[n].value, sizeof(in));
     memcpy(&in, buf, sizeof(in));
@@ -427,6 +500,9 @@ static int test_param_bignum(int n)
                                        NULL, 0);
     int ret = 0;
 
+    if (!TEST_int_eq(test_param_type_null(&param), 1))
+        return 0;
+
     param.data = bnbuf;
     param.data_size = sizeof(bnbuf);
 
@@ -458,6 +534,9 @@ static int test_param_signed_bignum(int n)
     OSSL_PARAM param = OSSL_PARAM_DEFN("bn", OSSL_PARAM_INTEGER, NULL, 0);
     int ret = 0;
 
+    if (!TEST_int_eq(test_param_type_null(&param), 1))
+        return 0;
+
     param.data = bnbuf;
     param.data_size = sizeof(bnbuf);
 
@@ -491,6 +570,9 @@ static int test_param_real(void)
     double p;
     OSSL_PARAM param = OSSL_PARAM_double("r", NULL);
 
+    if (!TEST_int_eq(test_param_type_null(&param), 1))
+        return 0;
+
     param.data = &p;
     return TEST_true(OSSL_PARAM_set_double(&param, 3.14159))
            && TEST_double_eq(p, 3.14159);