stack.c: add missing direct error reporting and improve coding style

Doing so, had to fix sloppiness in using the stack API in crypto/conf/conf_def.c,
ssl/ssl_ciph.c, ssl/statem/statem_srvr.c, and mostly in test/helpers/ssltestlib.c.

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Shane Lontis <shane.lontis@oracle.com>
Reviewed-by: David von Oheimb <david.von.oheimb@siemens.com>
(Merged from https://github.com/openssl/openssl/pull/18918)
This commit is contained in:
Dr. David von Oheimb 2022-08-07 07:08:28 +02:00 committed by Dr. David von Oheimb
parent 38ebfc3f5f
commit 30eba7f359
6 changed files with 89 additions and 30 deletions

View File

@ -296,7 +296,7 @@ static int def_load_bio(CONF *conf, BIO *in, long *line)
}
#endif
/* no more files in directory, continue with processing parent */
if ((parent = sk_BIO_pop(biosk)) == NULL) {
if (sk_BIO_num(biosk) < 1 || (parent = sk_BIO_pop(biosk)) == NULL) {
/* everything processed get out of the loop */
break;
} else {

View File

@ -22,8 +22,7 @@ OSSL_SAFE_MATH_SIGNED(int, int)
*/
static const int min_nodes = 4;
static const int max_nodes = SIZE_MAX / sizeof(void *) < INT_MAX
? (int)(SIZE_MAX / sizeof(void *))
: INT_MAX;
? (int)(SIZE_MAX / sizeof(void *)) : INT_MAX;
struct stack_st {
int num;
@ -33,7 +32,8 @@ struct stack_st {
OPENSSL_sk_compfunc comp;
};
OPENSSL_sk_compfunc OPENSSL_sk_set_cmp_func(OPENSSL_STACK *sk, OPENSSL_sk_compfunc c)
OPENSSL_sk_compfunc OPENSSL_sk_set_cmp_func(OPENSSL_STACK *sk,
OPENSSL_sk_compfunc c)
{
OPENSSL_sk_compfunc old = sk->comp;
@ -68,7 +68,8 @@ OPENSSL_STACK *OPENSSL_sk_dup(const OPENSSL_STACK *sk)
}
/* duplicate |sk->data| content */
if ((ret->data = OPENSSL_malloc(sizeof(*ret->data) * sk->num_alloc)) == NULL)
ret->data = OPENSSL_malloc(sizeof(*ret->data) * sk->num_alloc);
if (ret->data == NULL)
goto err;
memcpy(ret->data, sk->data, sizeof(void *) * sk->num);
return ret;
@ -80,8 +81,8 @@ OPENSSL_STACK *OPENSSL_sk_dup(const OPENSSL_STACK *sk)
}
OPENSSL_STACK *OPENSSL_sk_deep_copy(const OPENSSL_STACK *sk,
OPENSSL_sk_copyfunc copy_func,
OPENSSL_sk_freefunc free_func)
OPENSSL_sk_copyfunc copy_func,
OPENSSL_sk_freefunc free_func)
{
OPENSSL_STACK *ret;
int i;
@ -166,7 +167,7 @@ static ossl_inline int compute_growth(int target, int current)
return 0;
current = safe_muldiv_int(current, 8, 5, &err);
if (err)
if (err != 0)
return 0;
if (current >= max_nodes)
current = max_nodes;
@ -181,8 +182,10 @@ static int sk_reserve(OPENSSL_STACK *st, int n, int exact)
int num_alloc;
/* Check to see the reservation isn't exceeding the hard limit */
if (n > max_nodes - st->num)
if (n > max_nodes - st->num) {
ERR_raise(ERR_LIB_CRYPTO, CRYPTO_R_TOO_MANY_RECORDS);
return 0;
}
/* Figure out the new size */
num_alloc = st->num + n;
@ -207,15 +210,19 @@ static int sk_reserve(OPENSSL_STACK *st, int n, int exact)
if (num_alloc <= st->num_alloc)
return 1;
num_alloc = compute_growth(num_alloc, st->num_alloc);
if (num_alloc == 0)
if (num_alloc == 0) {
ERR_raise(ERR_LIB_CRYPTO, CRYPTO_R_TOO_MANY_RECORDS);
return 0;
}
} else if (num_alloc == st->num_alloc) {
return 1;
}
tmpdata = OPENSSL_realloc((void *)st->data, sizeof(void *) * num_alloc);
if (tmpdata == NULL)
if (tmpdata == NULL) {
ERR_raise(ERR_LIB_CRYPTO, ERR_R_MALLOC_FAILURE);
return 0;
}
st->data = tmpdata;
st->num_alloc = num_alloc;
@ -226,8 +233,10 @@ OPENSSL_STACK *OPENSSL_sk_new_reserve(OPENSSL_sk_compfunc c, int n)
{
OPENSSL_STACK *st = OPENSSL_zalloc(sizeof(OPENSSL_STACK));
if (st == NULL)
if (st == NULL) {
ERR_raise(ERR_LIB_CRYPTO, ERR_R_MALLOC_FAILURE);
return NULL;
}
st->comp = c;
@ -244,8 +253,10 @@ OPENSSL_STACK *OPENSSL_sk_new_reserve(OPENSSL_sk_compfunc c, int n)
int OPENSSL_sk_reserve(OPENSSL_STACK *st, int n)
{
if (st == NULL)
if (st == NULL) {
ERR_raise(ERR_LIB_CRYPTO, ERR_R_PASSED_NULL_PARAMETER);
return 0;
}
if (n < 0)
return 1;
@ -254,8 +265,14 @@ int OPENSSL_sk_reserve(OPENSSL_STACK *st, int n)
int OPENSSL_sk_insert(OPENSSL_STACK *st, const void *data, int loc)
{
if (st == NULL || st->num == max_nodes)
if (st == NULL) {
ERR_raise(ERR_LIB_CRYPTO, ERR_R_PASSED_NULL_PARAMETER);
return 0;
}
if (st->num == max_nodes) {
ERR_raise(ERR_LIB_CRYPTO, CRYPTO_R_TOO_MANY_RECORDS);
return 0;
}
if (!sk_reserve(st, 1, 0))
return 0;
@ -277,8 +294,8 @@ static ossl_inline void *internal_delete(OPENSSL_STACK *st, int loc)
const void *ret = st->data[loc];
if (loc != st->num - 1)
memmove(&st->data[loc], &st->data[loc + 1],
sizeof(st->data[0]) * (st->num - loc - 1));
memmove(&st->data[loc], &st->data[loc + 1],
sizeof(st->data[0]) * (st->num - loc - 1));
st->num--;
return (void *)ret;
@ -296,8 +313,15 @@ void *OPENSSL_sk_delete_ptr(OPENSSL_STACK *st, const void *p)
void *OPENSSL_sk_delete(OPENSSL_STACK *st, int loc)
{
if (st == NULL || loc < 0 || loc >= st->num)
if (st == NULL) {
ERR_raise(ERR_LIB_X509, ERR_R_PASSED_NULL_PARAMETER);
return NULL;
}
if (loc < 0 || loc >= st->num) {
ERR_raise_data(ERR_LIB_X509, ERR_R_PASSED_INVALID_ARGUMENT,
"loc=%d", loc);
return NULL;
}
return internal_delete(st, loc);
}
@ -381,21 +405,37 @@ int OPENSSL_sk_unshift(OPENSSL_STACK *st, const void *data)
void *OPENSSL_sk_shift(OPENSSL_STACK *st)
{
if (st == NULL || st->num == 0)
if (st == NULL) {
ERR_raise(ERR_LIB_X509, ERR_R_PASSED_NULL_PARAMETER);
return NULL;
}
if (st->num == 0) {
ERR_raise(ERR_LIB_X509, ERR_R_PASSED_INVALID_ARGUMENT);
return NULL;
}
return internal_delete(st, 0);
}
void *OPENSSL_sk_pop(OPENSSL_STACK *st)
{
if (st == NULL || st->num == 0)
if (st == NULL) {
ERR_raise(ERR_LIB_X509, ERR_R_PASSED_NULL_PARAMETER);
return NULL;
}
if (st->num == 0) {
ERR_raise(ERR_LIB_X509, ERR_R_PASSED_INVALID_ARGUMENT);
return NULL;
}
return internal_delete(st, st->num - 1);
}
void OPENSSL_sk_zero(OPENSSL_STACK *st)
{
if (st == NULL || st->num == 0)
if (st == NULL) {
ERR_raise(ERR_LIB_X509, ERR_R_PASSED_NULL_PARAMETER);
return;
}
if (st->num == 0)
return;
memset(st->data, 0, sizeof(*st->data) * st->num);
st->num = 0;
@ -428,15 +468,29 @@ int OPENSSL_sk_num(const OPENSSL_STACK *st)
void *OPENSSL_sk_value(const OPENSSL_STACK *st, int i)
{
if (st == NULL || i < 0 || i >= st->num)
if (st == NULL) {
ERR_raise(ERR_LIB_X509, ERR_R_PASSED_NULL_PARAMETER);
return NULL;
}
if (i < 0 || i >= st->num) {
ERR_raise_data(ERR_LIB_X509, ERR_R_PASSED_INVALID_ARGUMENT,
"i=%d", i);
return NULL;
}
return (void *)st->data[i];
}
void *OPENSSL_sk_set(OPENSSL_STACK *st, int i, const void *data)
{
if (st == NULL || i < 0 || i >= st->num)
if (st == NULL) {
ERR_raise(ERR_LIB_X509, ERR_R_PASSED_NULL_PARAMETER);
return NULL;
}
if (i < 0 || i >= st->num) {
ERR_raise_data(ERR_LIB_X509, ERR_R_PASSED_INVALID_ARGUMENT,
"i=%d", i);
return NULL;
}
st->data[i] = data;
st->sorted = 0;
return (void *)st->data[i];

View File

@ -532,7 +532,8 @@ int ssl_cipher_get_evp(SSL_CTX *ctx, const SSL_SESSION *s,
ctmp.id = s->compress_meth;
if (ssl_comp_methods != NULL) {
i = sk_SSL_COMP_find(ssl_comp_methods, &ctmp);
*comp = sk_SSL_COMP_value(ssl_comp_methods, i);
if (i >= 0)
*comp = sk_SSL_COMP_value(ssl_comp_methods, i);
}
/* If were only interested in comp then return success */
if ((enc == NULL) && (md == NULL))

View File

@ -5741,7 +5741,8 @@ static int ct_move_scts(STACK_OF(SCT) **dst, STACK_OF(SCT) *src,
}
}
while ((sct = sk_SCT_pop(src)) != NULL) {
while (sk_SCT_num(src) > 0) {
sct = sk_SCT_pop(src);
if (SCT_set_source(sct, origin) != 1)
goto err;

View File

@ -3586,7 +3586,7 @@ MSG_PROCESS_RETURN tls_process_client_certificate(SSL_CONNECTION *s,
}
X509_free(s->session->peer);
s->session->peer = sk_X509_shift(sk);
s->session->peer = sk_X509_num(sk) == 0 ? NULL: sk_X509_shift(sk);
s->session->verify_result = s->verify_result;
OSSL_STACK_OF_X509_free(s->session->peer_chain);

View File

@ -350,8 +350,9 @@ static int mempacket_test_read(BIO *bio, char *out, int outl)
unsigned int seq, offset, len, epoch;
BIO_clear_retry_flags(bio);
thispkt = sk_MEMPACKET_value(ctx->pkts, 0);
if (thispkt == NULL || thispkt->num != ctx->currpkt) {
if (sk_MEMPACKET_num(ctx->pkts) <= 0
|| (thispkt = sk_MEMPACKET_value(ctx->pkts, 0)) == NULL
|| thispkt->num != ctx->currpkt) {
/* Probably run out of data */
BIO_set_retry_read(bio);
return -1;
@ -585,7 +586,8 @@ int mempacket_test_inject(BIO *bio, const char *in, int inl, int pktnum,
thispkt->type = type;
}
for (i = 0; (looppkt = sk_MEMPACKET_value(ctx->pkts, i)) != NULL; i++) {
for (i = 0; i < sk_MEMPACKET_num(ctx->pkts); i++) {
looppkt = sk_MEMPACKET_value(ctx->pkts, i);
/* Check if we found the right place to insert this packet */
if (looppkt->num > thispkt->num) {
if (sk_MEMPACKET_insert(ctx->pkts, thispkt, i) == 0)
@ -601,8 +603,9 @@ int mempacket_test_inject(BIO *bio, const char *in, int inl, int pktnum,
ctx->lastpkt++;
do {
i++;
nextpkt = sk_MEMPACKET_value(ctx->pkts, i);
if (nextpkt != NULL && nextpkt->num == ctx->lastpkt)
if (i < sk_MEMPACKET_num(ctx->pkts)
&& (nextpkt = sk_MEMPACKET_value(ctx->pkts, i)) != NULL
&& nextpkt->num == ctx->lastpkt)
ctx->lastpkt++;
else
return inl;