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

This partially reverts commit 30eba7f35983a917f1007bce45040c0af3442e42.
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)

2 files changed, 9 insertions, 36 deletions

diff --git a/crypto/conf/conf_def.c b/crypto/conf/conf_def.c
index b6b56cd967..5e81d9e941 100644
--- a/crypto/conf/conf_def.c
+++ b/crypto/conf/conf_def.c
@@ -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 {
diff --git a/crypto/stack/stack.c b/crypto/stack/stack.c
index 7e1c24515c..dc1c7d36d0 100644
--- a/crypto/stack/stack.c
+++ b/crypto/stack/stack.c
@@ -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);
-        return NULL;
-    }
-    if (st->num == 0) {
-        ERR_raise(ERR_LIB_X509, ERR_R_PASSED_INVALID_ARGUMENT);
+    if (st == NULL || st->num == 0)
         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);
-        return NULL;
-    }
-    if (st->num == 0) {
-        ERR_raise(ERR_LIB_X509, ERR_R_PASSED_INVALID_ARGUMENT);
+    if (st == NULL || st->num == 0)
         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);
-        return NULL;
-    }
-    if (i < 0 || i >= st->num) {
-        ERR_raise_data(ERR_LIB_X509, ERR_R_PASSED_INVALID_ARGUMENT,
-                       "i=%d", i);
+    if (st == NULL || i < 0 || i >= st->num)
         return NULL;
-    }
     return (void *)st->data[i];
 }