stack: Do not add error if pop/shift/value accesses outside of the stack

This partially reverts commit 30eba7f359.
This is legitimate use of the stack functions and no error
should be reported apart from the NULL return value.

Fixes #19389

Reviewed-by: Todd Short <todd.short@me.com>
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/19400)
This commit is contained in:
Tomas Mraz 2022-10-12 10:36:20 +02:00
parent fba324204f
commit a8086e6bfc
5 changed files with 14 additions and 44 deletions

View File

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

View File

@ -297,6 +297,9 @@ void *OPENSSL_sk_delete_ptr(OPENSSL_STACK *st, const void *p)
{
int i;
if (st == NULL)
return NULL;
for (i = 0; i < st->num; i++)
if (st->data[i] == p)
return internal_delete(st, i);
@ -305,15 +308,8 @@ void *OPENSSL_sk_delete_ptr(OPENSSL_STACK *st, const void *p)
void *OPENSSL_sk_delete(OPENSSL_STACK *st, int loc)
{
if (st == NULL) {
ERR_raise(ERR_LIB_X509, ERR_R_PASSED_NULL_PARAMETER);
if (st == NULL || loc < 0 || loc >= st->num)
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);
}
@ -397,37 +393,21 @@ int OPENSSL_sk_unshift(OPENSSL_STACK *st, const void *data)
void *OPENSSL_sk_shift(OPENSSL_STACK *st)
{
if (st == NULL) {
ERR_raise(ERR_LIB_X509, ERR_R_PASSED_NULL_PARAMETER);
if (st == NULL || st->num == 0)
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) {
ERR_raise(ERR_LIB_X509, ERR_R_PASSED_NULL_PARAMETER);
if (st == NULL || st->num == 0)
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) {
ERR_raise(ERR_LIB_X509, ERR_R_PASSED_NULL_PARAMETER);
return;
}
if (st->num == 0)
if (st == NULL || st->num == 0)
return;
memset(st->data, 0, sizeof(*st->data) * st->num);
st->num = 0;
@ -460,15 +440,8 @@ int OPENSSL_sk_num(const OPENSSL_STACK *st)
void *OPENSSL_sk_value(const OPENSSL_STACK *st, int i)
{
if (st == NULL) {
ERR_raise(ERR_LIB_X509, ERR_R_PASSED_NULL_PARAMETER);
if (st == NULL || i < 0 || i >= st->num)
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];
}

View File

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

View File

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

View File

@ -349,8 +349,7 @@ static int mempacket_test_read(BIO *bio, char *out, int outl)
unsigned int seq, offset, len, epoch;
BIO_clear_retry_flags(bio);
if (sk_MEMPACKET_num(ctx->pkts) <= 0
|| (thispkt = sk_MEMPACKET_value(ctx->pkts, 0)) == NULL
if ((thispkt = sk_MEMPACKET_value(ctx->pkts, 0)) == NULL
|| thispkt->num != ctx->currpkt) {
/* Probably run out of data */
BIO_set_retry_read(bio);
@ -603,9 +602,8 @@ int mempacket_test_inject(BIO *bio, const char *in, int inl, int pktnum,
ctx->lastpkt++;
do {
i++;
if (i < sk_MEMPACKET_num(ctx->pkts)
&& (nextpkt = sk_MEMPACKET_value(ctx->pkts, i)) != NULL
&& nextpkt->num == ctx->lastpkt)
nextpkt = sk_MEMPACKET_value(ctx->pkts, i);
if (nextpkt != NULL && nextpkt->num == ctx->lastpkt)
ctx->lastpkt++;
else
return inl;