From: Neil Horman Date: Mon, 18 Dec 2023 15:55:25 +0000 (-0500) Subject: Check appropriate OSSL_PARAM_get_* functions for NULL X-Git-Tag: openssl-3.3.0-alpha1~339 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=806bbafe2df5b699feac6ef26e50c14e701950cf;p=thirdparty%2Fopenssl.git 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 Reviewed-by: Richard Levitte Reviewed-by: Tomas Mraz (Merged from https://github.com/openssl/openssl/pull/23083) --- diff --git a/crypto/params.c b/crypto/params.c index f2582b0927a..425c402fd53 100644 --- a/crypto/params.c +++ b/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; } diff --git a/test/params_api_test.c b/test/params_api_test.c index f4227b0012f..941b05c4d38 100644 --- a/test/params_api_test.c +++ b/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(¶m), 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(¶m), 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(¶m), 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(¶m), 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(¶m), 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(¶m), 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(¶m), 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(¶m), 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(¶m), 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(¶m), 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(¶m), 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(¶m), 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(¶m), 1)) + return 0; + param.data = &p; return TEST_true(OSSL_PARAM_set_double(¶m, 3.14159)) && TEST_double_eq(p, 3.14159);