stack: use a copy thunk for typed stack deep copies

typed safestack wrappers pass type-specific copy callbacks such as
TYPE *(*)(const TYPE *) to OPENSSL_sk_deep_copy().  The generic stack code
then called those callbacks through OPENSSL_sk_copyfunc,
void *(*)(const void *), which is an incompatible function pointer type and
triggers UBSan.

Add an OPENSSL_sk_copyfunc_thunk and store it on typed stacks, mirroring the
existing compare/free thunk pattern.  Generated safestack helpers now install
a per-type copy thunk when constructing a stack, and internal_copy() uses that
thunk when deep-copying typed stacks.  This preserves the generic stack API
while ensuring typed copy callbacks are invoked through their real signature.

Fixes: https://github.com/openssl/project/issues/1951
Signed-off-by: Nikola Pajkovsky <nikolap@openssl.org>
Reviewed-by: Eugene Syromiatnikov <esyr@openssl.org>
Reviewed-by: Neil Horman <nhorman@openssl.org>
MergeDate: Wed May 20 15:53:45 2026
(Merged from https://github.com/openssl/openssl/pull/31151)

diff --git a/crypto/stack/stack.c b/crypto/stack/stack.c
index bd3c6f54ef1107e0c71c747426910ac63a7cea9e..5799bf9d7ca9fa75174cd688c84742ee2f5307b0 100644 (file)
--- a/crypto/stack/stack.c
+++ b/crypto/stack/stack.c
@@ -33,6 +33,7 @@ struct stack_st {
     OPENSSL_sk_compfunc comp;
     int (*cmp_thunk)(OPENSSL_sk_compfunc, const void *, const void *);
     OPENSSL_sk_freefunc_thunk free_thunk;
+    OPENSSL_sk_copyfunc_thunk copy_thunk;
 };
 
 OPENSSL_sk_compfunc OPENSSL_sk_set_cmp_func(OPENSSL_STACK *sk,
@@ -88,7 +89,12 @@ static OPENSSL_STACK *internal_copy(const OPENSSL_STACK *sk,
         for (i = 0; i < ret->num; ++i) {
             if (sk->data[i] == NULL)
                 continue;
-            if ((ret->data[i] = copy_func(sk->data[i])) == NULL) {
+            if (ret->copy_thunk != NULL)
+                ret->data[i] = ret->copy_thunk(copy_func, sk->data[i]);
+            else
+                ret->data[i] = copy_func(sk->data[i]);
+
+            if (ret->data[i] == NULL) {
                 while (--i >= 0)
                     free_with_thunk(ret, free_func, ret->data[i]);
                 goto err;
@@ -266,6 +272,14 @@ OPENSSL_STACK *OPENSSL_sk_set_cmp_thunks(OPENSSL_STACK *st, int (*c_thunk)(int (
     return st;
 }
 
+OPENSSL_STACK *OPENSSL_sk_set_copy_thunks(OPENSSL_STACK *st, OPENSSL_sk_copyfunc_thunk cp_thunk)
+{
+    if (st != NULL)
+        st->copy_thunk = cp_thunk;
+
+    return st;
+}
+
 int OPENSSL_sk_insert(OPENSSL_STACK *st, const void *data, int loc)
 {
     int cmp_ret;

diff --git a/doc/man3/DEFINE_STACK_OF.pod b/doc/man3/DEFINE_STACK_OF.pod
index a9d678bacccd22b9241c1f8637a96977e81c3f3d..5f4fcaa487d3781cee26e72ef63af4ebefe67b5a 100644 (file)
--- a/doc/man3/DEFINE_STACK_OF.pod
+++ b/doc/man3/DEFINE_STACK_OF.pod
@@ -16,7 +16,8 @@ OPENSSL_sk_dup, OPENSSL_sk_find, OPENSSL_sk_find_ex, OPENSSL_sk_find_all,
 OPENSSL_sk_free, OPENSSL_sk_insert, OPENSSL_sk_is_sorted, OPENSSL_sk_new,
 OPENSSL_sk_new_null, OPENSSL_sk_new_reserve, OPENSSL_sk_num, OPENSSL_sk_pop,
 OPENSSL_sk_pop_free, OPENSSL_sk_push, OPENSSL_sk_reserve, OPENSSL_sk_set,
-OPENSSL_sk_set_thunks, OPENSSL_sk_set_cmp_thunks, OPENSSL_sk_set_cmp_func, OPENSSL_sk_shift,
+OPENSSL_sk_set_thunks, OPENSSL_sk_set_cmp_thunks, OPENSSL_sk_set_copy_thunks,
+OPENSSL_sk_set_cmp_func, OPENSSL_sk_shift,
 OPENSSL_sk_sort, OPENSSL_sk_unshift, OPENSSL_sk_value, OPENSSL_sk_zero
 - stack container
 
@@ -240,11 +241,13 @@ OPENSSL_sk_free(), OPENSSL_sk_insert(), OPENSSL_sk_is_sorted(),
 OPENSSL_sk_new(), OPENSSL_sk_new_null(), OPENSSL_sk_new_reserve(),
 OPENSSL_sk_num(), OPENSSL_sk_pop(), OPENSSL_sk_pop_free(), OPENSSL_sk_push(),
 OPENSSL_sk_reserve(), OPENSSL_sk_set(), OPENSSL_sk_set_cmp_func(),
-OPENSSL_sk_set_thunks(), OPENSSL_sk_set_cmp_thunks(), OPENSSL_sk_shift(), OPENSSL_sk_sort(),
+OPENSSL_sk_set_thunks(), OPENSSL_sk_set_cmp_thunks(),
+OPENSSL_sk_set_copy_thunks(), OPENSSL_sk_shift(), OPENSSL_sk_sort(),
 OPENSSL_sk_unshift(), OPENSSL_sk_value(), OPENSSL_sk_zero().
 
-OPENSSL_sk_set_thunks() and OPENSSL_sk_set_cmp_thunks(), while public by necessity, are actually
-internal and should not be used.
+OPENSSL_sk_set_thunks(), OPENSSL_sk_set_cmp_thunks(), and
+OPENSSL_sk_set_copy_thunks(), while public by necessity, are actually internal
+and should not be used.
 
 =head1 RETURN VALUES
 
@@ -303,7 +306,9 @@ was changed to return 0 in this condition as for other errors.
 
 OPENSSL_sk_set_thunks() was added in OpenSSL 3.6.0.
 
-OPENSSL_sk_set_cmp_thunks() was added in OpenSSL 4.0.0
+OPENSSL_sk_set_cmp_thunks() was added in OpenSSL 4.0.0.
+
+OPENSSL_sk_set_copy_thunks() was added in OpenSSL 4.1.0.
 
 =head1 COPYRIGHT
 

diff --git a/include/openssl/safestack.h.in b/include/openssl/safestack.h.in
index 67e0448252ae5d21b6b2884b084e8c10b006a2a2..a4c178d1897e02e3f3ccf4336babc7271d5eb195 100644 (file)
--- a/include/openssl/safestack.h.in
+++ b/include/openssl/safestack.h.in
@@ -46,6 +46,11 @@ extern "C" {
         sk_##t1##_freefunc freefunc = (sk_##t1##_freefunc)freefunc_arg;                                                  \
         freefunc((t3 *)ptr);                                                                                             \
     }                                                                                                                    \
+    static ossl_inline void *sk_##t1##_copyfunc_thunk(OPENSSL_sk_copyfunc copyfunc_arg, const void *ptr)                 \
+    {                                                                                                                    \
+        sk_##t1##_copyfunc copyfunc = (sk_##t1##_copyfunc)copyfunc_arg;                                                  \
+        return (void *)copyfunc((const t3 *)ptr);                                                                        \
+    }                                                                                                                    \
     static ossl_inline int sk_##t1##_cmpfunc_thunk(int (*cmp)(const void *, const void *), const void *a, const void *b) \
     {                                                                                                                    \
         int (*realcmp)(const t3 *const *a, const t3 *const *b) = (int (*)(const t3 *const *a, const t3 *const *b))(cmp); \
@@ -89,6 +94,11 @@ extern "C" {
         sk_##t1##_freefunc freefunc = (sk_##t1##_freefunc)freefunc_arg;                                                    \
         freefunc((t3 *)ptr);                                                                                               \
     }                                                                                                                      \
+    static ossl_inline void *sk_##t1##_copyfunc_thunk(OPENSSL_sk_copyfunc copyfunc_arg, const void *ptr)                   \
+    {                                                                                                                      \
+        sk_##t1##_copyfunc copyfunc = (sk_##t1##_copyfunc)copyfunc_arg;                                                    \
+        return (void *)copyfunc((const t3 *)ptr);                                                                          \
+    }                                                                                                                      \
     static ossl_inline int sk_##t1##_cmpfunc_thunk(int (*cmp)(const void *, const void *), const void *a, const void *b)   \
     {                                                                                                                      \
         int (*realcmp)(const t3 *const *a, const t3 *const *b) = (int (*)(const t3 *const *a, const t3 *const *b))(cmp);   \
@@ -112,6 +122,7 @@ extern "C" {
                                                                                                                            \
         f_thunk = (OPENSSL_sk_freefunc_thunk)sk_##t1##_freefunc_thunk;                                                     \
         OPENSSL_sk_set_cmp_thunks(ret, sk_##t1##_cmpfunc_thunk);                                                           \
+        OPENSSL_sk_set_copy_thunks(ret, sk_##t1##_copyfunc_thunk);                                                         \
         return (STACK_OF(t1) *)OPENSSL_sk_set_thunks(ret, f_thunk);                                                        \
     }                                                                                                                      \
     static ossl_unused ossl_inline STACK_OF(t1) *sk_##t1##_new_null(void)                                                  \
@@ -121,6 +132,7 @@ extern "C" {
                                                                                                                            \
         f_thunk = (OPENSSL_sk_freefunc_thunk)sk_##t1##_freefunc_thunk;                                                     \
         OPENSSL_sk_set_cmp_thunks(ret, sk_##t1##_cmpfunc_thunk);                                                           \
+        OPENSSL_sk_set_copy_thunks(ret, sk_##t1##_copyfunc_thunk);                                                         \
         return (STACK_OF(t1) *)OPENSSL_sk_set_thunks(ret, f_thunk);                                                        \
     }                                                                                                                      \
     static ossl_unused ossl_inline STACK_OF(t1) *sk_##t1##_new_reserve(sk_##t1##_compfunc compare, int n)                  \
@@ -130,6 +142,7 @@ extern "C" {
                                                                                                                            \
         f_thunk = (OPENSSL_sk_freefunc_thunk)sk_##t1##_freefunc_thunk;                                                     \
         OPENSSL_sk_set_cmp_thunks(ret, sk_##t1##_cmpfunc_thunk);                                                           \
+        OPENSSL_sk_set_copy_thunks(ret, sk_##t1##_copyfunc_thunk);                                                         \
         return (STACK_OF(t1) *)OPENSSL_sk_set_thunks(ret, f_thunk);                                                        \
     }                                                                                                                      \
     static ossl_unused ossl_inline int sk_##t1##_reserve(STACK_OF(t1) *sk, int n)                                          \

diff --git a/include/openssl/stack.h b/include/openssl/stack.h
index 67e4d258972e6b1664e6ebc5831bd76ad41c41df..7cab36c4f647949cec6fa5e8acd83bc4a5e81b96 100644 (file)
--- a/include/openssl/stack.h
+++ b/include/openssl/stack.h
@@ -26,6 +26,7 @@ typedef int (*OPENSSL_sk_compfunc)(const void *, const void *);
 typedef void (*OPENSSL_sk_freefunc)(void *);
 typedef void (*OPENSSL_sk_freefunc_thunk)(OPENSSL_sk_freefunc, void *);
 typedef void *(*OPENSSL_sk_copyfunc)(const void *);
+typedef void *(*OPENSSL_sk_copyfunc_thunk)(OPENSSL_sk_copyfunc, const void *);
 
 int OPENSSL_sk_num(const OPENSSL_STACK *);
 void *OPENSSL_sk_value(const OPENSSL_STACK *, int);
@@ -37,6 +38,7 @@ OPENSSL_STACK *OPENSSL_sk_new_null(void);
 OPENSSL_STACK *OPENSSL_sk_new_reserve(OPENSSL_sk_compfunc c, int n);
 OPENSSL_STACK *OPENSSL_sk_set_thunks(OPENSSL_STACK *st, OPENSSL_sk_freefunc_thunk f_thunk);
 OPENSSL_STACK *OPENSSL_sk_set_cmp_thunks(OPENSSL_STACK *st, int (*c_thunk)(int (*)(const void *, const void *), const void *, const void *));
+OPENSSL_STACK *OPENSSL_sk_set_copy_thunks(OPENSSL_STACK *st, OPENSSL_sk_copyfunc_thunk cp_thunk);
 int OPENSSL_sk_reserve(OPENSSL_STACK *st, int n);
 void OPENSSL_sk_free(OPENSSL_STACK *);
 void OPENSSL_sk_pop_free(OPENSSL_STACK *st, OPENSSL_sk_freefunc func);

diff --git a/test/stack_test.c b/test/stack_test.c
index aa29e6c822fffd983952eb9aef27c38e9b8bd4e7..c74cca216779d8acc6413cf4d2ee215428528d85 100644 (file)
--- a/test/stack_test.c
+++ b/test/stack_test.c
@@ -1,5 +1,5 @@
 /*
- * Copyright 2017-2025 The OpenSSL Project Authors. All Rights Reserved.
+ * Copyright 2017-2026 The OpenSSL Project Authors. All Rights Reserved.
  * Copyright (c) 2017, Oracle and/or its affiliates.  All rights reserved.
  *
  * Licensed under the Apache License 2.0 (the "License").  You may not use
@@ -339,6 +339,16 @@ static void SS_free(SS *p)
     OPENSSL_free(p);
 }
 
+static char *string_copy(const char *p)
+{
+    return OPENSSL_strdup(p);
+}
+
+static void string_free(char *p)
+{
+    OPENSSL_free(p);
+}
+
 static int test_SS_stack(void)
 {
     STACK_OF(SS) *s = sk_SS_new_null();
@@ -413,6 +423,55 @@ end:
     return testresult;
 }
 
+static int test_OPENSSL_STRING_deep_copy_mfail(void)
+{
+    STACK_OF(OPENSSL_STRING) *s = sk_OPENSSL_STRING_new_null();
+    STACK_OF(OPENSSL_STRING) *r = NULL;
+    static const char *strings[] = {
+        "alpha", "beta", "gamma"
+    };
+    char *p;
+    int i;
+    int testresult = 0;
+
+    if (!TEST_ptr(s))
+        goto end;
+
+    for (i = 0; i < (int)OSSL_NELEM(strings); i++) {
+        p = OPENSSL_strdup(strings[i]);
+        if (!TEST_ptr(p)
+            || !TEST_int_eq(sk_OPENSSL_STRING_push(s, p), i + 1)) {
+            OPENSSL_free(p);
+            goto end;
+        }
+    }
+
+    MFAIL_start();
+    r = sk_OPENSSL_STRING_deep_copy(s, string_copy, string_free);
+    MFAIL_end();
+
+    if (r == NULL)
+        goto end;
+
+    if (!TEST_int_eq(sk_OPENSSL_STRING_num(r), sk_OPENSSL_STRING_num(s)))
+        goto end;
+
+    for (i = 0; i < sk_OPENSSL_STRING_num(s); i++) {
+        char *src = sk_OPENSSL_STRING_value(s, i);
+        char *dst = sk_OPENSSL_STRING_value(r, i);
+
+        if (!TEST_ptr_ne(dst, src)
+            || !TEST_str_eq(dst, src))
+            goto end;
+    }
+
+    testresult = 1;
+end:
+    sk_OPENSSL_STRING_pop_free(r, string_free);
+    sk_OPENSSL_STRING_pop_free(s, string_free);
+    return testresult;
+}
+
 static int test_SU_stack(void)
 {
     STACK_OF(SU) *s = sk_SU_new_null();
@@ -455,5 +514,6 @@ int setup_tests(void)
     ADD_ALL_TESTS(test_uchar_stack, 4);
     ADD_TEST(test_SS_stack);
     ADD_TEST(test_SU_stack);
+    ADD_MFAIL_TEST(test_OPENSSL_STRING_deep_copy_mfail);
     return 1;
 }

diff --git a/util/libcrypto.num b/util/libcrypto.num
index dbaac8b37fa77c8f8b6bb751ea499f50da24275f..659eb5cbf45c4504a84bc59dfb4f9c451204eaa4 100644 (file)
--- a/util/libcrypto.num
+++ b/util/libcrypto.num
@@ -5720,3 +5720,4 @@ CRYPTO_atomic_load_ptr                  ? 4_1_0   EXIST::FUNCTION:
 CRYPTO_atomic_store_ptr                 ?      4_1_0   EXIST::FUNCTION:
 CRYPTO_atomic_cmp_exch_ptr              ?      4_1_0   EXIST::FUNCTION:
 EVP_EC_affine2oct                       ?      4_1_0   EXIST::FUNCTION:
+OPENSSL_sk_set_copy_thunks              ?      4_1_0   EXIST::FUNCTION:

diff --git a/util/perl/OpenSSL/stackhash.pm b/util/perl/OpenSSL/stackhash.pm
index cab5738d187a6e3d32f9d6a93a47dd2be66096d3..39f71e1ce7175dd412bf7a05dbffaa7d01113264 100644 (file)
--- a/util/perl/OpenSSL/stackhash.pm
+++ b/util/perl/OpenSSL/stackhash.pm
@@ -28,9 +28,9 @@ sub generate_stack_macros_int {
 SKM_DEFINE_STACK_OF_INTERNAL(${nametype}, ${realtype}, ${plaintype})
 #define sk_${nametype}_num(sk) OPENSSL_sk_num(ossl_check_const_${nametype}_sk_type(sk))
 #define sk_${nametype}_value(sk, idx) ((${realtype} *)OPENSSL_sk_value(ossl_check_const_${nametype}_sk_type(sk), (idx)))
-#define sk_${nametype}_new(cmp) ((STACK_OF(${nametype}) *)OPENSSL_sk_set_cmp_thunks(OPENSSL_sk_new(ossl_check_${nametype}_compfunc_type(cmp)), sk_${nametype}_cmpfunc_thunk))
-#define sk_${nametype}_new_null() ((STACK_OF(${nametype}) *)OPENSSL_sk_set_thunks(OPENSSL_sk_new_null(), sk_${nametype}_freefunc_thunk))
-#define sk_${nametype}_new_reserve(cmp, n) ((STACK_OF(${nametype}) *)OPENSSL_sk_set_cmp_thunks(OPENSSL_sk_new_reserve(ossl_check_${nametype}_compfunc_type(cmp), (n)), sk_${nametype}_cmpfunc_thunk))
+#define sk_${nametype}_new(cmp) ((STACK_OF(${nametype}) *)OPENSSL_sk_set_copy_thunks(OPENSSL_sk_set_cmp_thunks(OPENSSL_sk_new(ossl_check_${nametype}_compfunc_type(cmp)), sk_${nametype}_cmpfunc_thunk), sk_${nametype}_copyfunc_thunk))
+#define sk_${nametype}_new_null() ((STACK_OF(${nametype}) *)OPENSSL_sk_set_thunks(OPENSSL_sk_set_copy_thunks(OPENSSL_sk_new_null(), sk_${nametype}_copyfunc_thunk), sk_${nametype}_freefunc_thunk))
+#define sk_${nametype}_new_reserve(cmp, n) ((STACK_OF(${nametype}) *)OPENSSL_sk_set_copy_thunks(OPENSSL_sk_set_cmp_thunks(OPENSSL_sk_new_reserve(ossl_check_${nametype}_compfunc_type(cmp), (n)), sk_${nametype}_cmpfunc_thunk), sk_${nametype}_copyfunc_thunk))
 #define sk_${nametype}_reserve(sk, n) OPENSSL_sk_reserve(ossl_check_${nametype}_sk_type(sk), (n))
 #define sk_${nametype}_free(sk) OPENSSL_sk_free(ossl_check_${nametype}_sk_type(sk))
 #define sk_${nametype}_zero(sk) OPENSSL_sk_zero(ossl_check_${nametype}_sk_type(sk))