Skip to content

Commit

Permalink
Check appropriate OSSL_PARAM_get_* functions for NULL
Browse files Browse the repository at this point in the history 
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 #23083)
  • Loading branch information
@nhorman @t8m
nhorman authored and t8m committed Jan 9, 2024
1 parent 858c7bc commit 806bbaf
Show file tree
Hide file tree
Showing 2 changed files with 113 additions and 2 deletions.
33 changes: 31 additions & 2 deletions crypto/params.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
}
Expand Down
82 changes: 82 additions & 0 deletions test/params_api_test.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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));
Expand All @@ -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));
Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit 806bbaf

Please sign in to comment.