preserve stack thunks across sk_TYPE_dup

ensure typed stack duplicates keep their compare, copy, and free thunks so
later operations do not call typed callbacks through erased void * function
pointer signatures.

Signed-off-by: Nikola Pajkovsky <nikolap@openssl.org>
Reviewed-by: Neil Horman <nhorman@openssl.org>
Reviewed-by: Milan Broz <mbroz@openssl.org>
MergeDate: Thu Jun 25 12:12:48 2026
(Merged from https://github.com/openssl/openssl/pull/31523)

diff --git a/include/openssl/safestack.h.in b/include/openssl/safestack.h.in
index a4c178d1897e02e3f3ccf4336babc7271d5eb195..64ac28be0494d40b31b0a80e1a43c9cb25e9486c 100644 (file)
--- a/include/openssl/safestack.h.in
+++ b/include/openssl/safestack.h.in
@@ -222,15 +222,27 @@ extern "C" {
     }                                                                                                                      \
     static ossl_unused ossl_inline STACK_OF(t1) *sk_##t1##_dup(const STACK_OF(t1) *sk)                                     \
     {                                                                                                                      \
-        return (STACK_OF(t1) *)OPENSSL_sk_dup((const OPENSSL_STACK *)sk);                                                  \
+        OPENSSL_STACK *ret = OPENSSL_sk_dup((const OPENSSL_STACK *)sk);                                                    \
+        OPENSSL_sk_freefunc_thunk f_thunk;                                                                                 \
+                                                                                                                           \
+        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##_deep_copy(const STACK_OF(t1) *sk,                               \
         sk_##t1##_copyfunc copyfunc,                                                                                       \
         sk_##t1##_freefunc freefunc)                                                                                       \
     {                                                                                                                      \
-        return (STACK_OF(t1) *)OPENSSL_sk_deep_copy((const OPENSSL_STACK *)sk,                                             \
+        OPENSSL_STACK *ret = OPENSSL_sk_deep_copy((const OPENSSL_STACK *)sk,                                               \
             (OPENSSL_sk_copyfunc)copyfunc,                                                                                 \
             (OPENSSL_sk_freefunc)freefunc);                                                                                \
+        OPENSSL_sk_freefunc_thunk f_thunk;                                                                                 \
+                                                                                                                           \
+        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 sk_##t1##_compfunc sk_##t1##_set_cmp_func(STACK_OF(t1) *sk, sk_##t1##_compfunc compare) \
     {                                                                                                                      \

diff --git a/test/stack_test.c b/test/stack_test.c
index 848c843931f9dc582003a7cf1861ae38351c8714..16d72f809e46e6fea8c238efb612f7fd5a645f7f 100644 (file)
--- a/test/stack_test.c
+++ b/test/stack_test.c
@@ -344,11 +344,35 @@ static char *string_copy(const char *p)
     return OPENSSL_strdup(p);
 }
 
+static int string_cmp(const char *const *a, const char *const *b)
+{
+    return strcmp(*a, *b);
+}
+
 static void string_free(char *p)
 {
     OPENSSL_free(p);
 }
 
+static int push_string(STACK_OF(OPENSSL_STRING) *s, const char *str)
+{
+    char *p = OPENSSL_strdup(str);
+    int expected = sk_OPENSSL_STRING_num(s) + 1;
+    int pushed;
+
+    if (!TEST_ptr(p))
+        return 0;
+
+    pushed = sk_OPENSSL_STRING_push(s, p);
+    if (!TEST_int_eq(pushed, expected)) {
+        if (pushed <= 0)
+            OPENSSL_free(p);
+        return 0;
+    }
+
+    return 1;
+}
+
 static int test_SS_stack(void)
 {
     STACK_OF(TST_SS) *s = sk_TST_SS_new_null();
@@ -508,11 +532,88 @@ end:
     return testresult;
 }
 
+static int test_STRING_STACK_stack_ubsan(int idx)
+{
+    STACK_OF(OPENSSL_STRING) *s = NULL;
+    STACK_OF(OPENSSL_STRING) *t = NULL;
+    STACK_OF(OPENSSL_STRING) *u = NULL;
+    int testresult = 0;
+
+    switch (idx) {
+    case 0:
+        s = sk_OPENSSL_STRING_new_null();
+        break;
+    case 1:
+        s = sk_OPENSSL_STRING_new(string_cmp);
+        break;
+    case 2:
+        s = sk_OPENSSL_STRING_new_reserve(string_cmp, 2);
+        break;
+    case 3:
+        s = sk_OPENSSL_STRING_deep_copy(NULL, string_copy, string_free);
+        break;
+    case 4:
+        t = sk_OPENSSL_STRING_new(string_cmp);
+        if (!TEST_ptr(t))
+            goto end;
+        if (!push_string(t, "b")
+            || !push_string(t, "a"))
+            goto end;
+        s = sk_OPENSSL_STRING_dup(t);
+        if (s != NULL) {
+            sk_OPENSSL_STRING_free(t);
+            t = NULL;
+        }
+        break;
+    case 5:
+        t = sk_OPENSSL_STRING_deep_copy(NULL, string_copy, string_free);
+        if (!TEST_ptr(t))
+            goto end;
+        if (!push_string(t, "b")
+            || !push_string(t, "a"))
+            goto end;
+        s = sk_OPENSSL_STRING_deep_copy(t, string_copy, string_free);
+        break;
+    default:
+        goto end;
+    }
+    if (!TEST_ptr(s))
+        goto end;
+
+    if (idx < 4) {
+        if (!push_string(s, "b")
+            || !push_string(s, "a"))
+            goto end;
+    }
+
+    sk_OPENSSL_STRING_set_cmp_func(s, string_cmp);
+    sk_OPENSSL_STRING_sort(s);
+    if (!TEST_str_eq(sk_OPENSSL_STRING_value(s, 0), "a")
+        || !TEST_int_eq(sk_OPENSSL_STRING_find(s, "b"), 1))
+        goto end;
+
+    u = sk_OPENSSL_STRING_deep_copy(s, string_copy, string_free);
+    if (!TEST_ptr(u)
+        || !TEST_int_eq(sk_OPENSSL_STRING_num(u), 2)
+        || !TEST_ptr_ne(sk_OPENSSL_STRING_value(u, 0),
+            sk_OPENSSL_STRING_value(s, 0))
+        || !TEST_str_eq(sk_OPENSSL_STRING_value(u, 0), "a"))
+        goto end;
+
+    testresult = 1;
+end:
+    sk_OPENSSL_STRING_pop_free(u, string_free);
+    sk_OPENSSL_STRING_pop_free(s, string_free);
+    sk_OPENSSL_STRING_pop_free(t, string_free);
+    return testresult;
+}
+
 int setup_tests(void)
 {
     ADD_ALL_TESTS(test_int_stack, 4);
     ADD_ALL_TESTS(test_uchar_stack, 4);
     ADD_TEST(test_SS_stack);
+    ADD_ALL_TESTS(test_STRING_STACK_stack_ubsan, 6);
     ADD_TEST(test_SU_stack);
     ADD_MFAIL_TEST(test_OPENSSL_STRING_deep_copy_mfail);
     return 1;

diff --git a/util/perl/OpenSSL/stackhash.pm b/util/perl/OpenSSL/stackhash.pm
index e49e2d4defe53644c38756b14a0a68035c45d356..b3446f77478f4c8303c4ac970c8be8e0ecfb6ed5 100644 (file)
--- a/util/perl/OpenSSL/stackhash.pm
+++ b/util/perl/OpenSSL/stackhash.pm
@@ -69,12 +69,25 @@ SKM_DEFINE_STACK_OF_INTERNAL(${nametype}, ${realtype}, ${plaintype})
 #define sk_${nametype}_find_all(sk, ptr, pnum) OPENSSL_sk_find_all(ossl_check_${nametype}_sk_type(sk), ossl_check_${nametype}_type(ptr), pnum)
 #define sk_${nametype}_sort(sk) OPENSSL_sk_sort(ossl_check_${nametype}_sk_type(sk))
 #define sk_${nametype}_is_sorted(sk) OPENSSL_sk_is_sorted(ossl_check_const_${nametype}_sk_type(sk))
-#define sk_${nametype}_dup(sk) ((STACK_OF(${nametype}) *)OPENSSL_sk_dup(ossl_check_const_${nametype}_sk_type(sk)))
+#define sk_${nametype}_dup(sk) \\
+    ((STACK_OF(${nametype}) *)OPENSSL_sk_set_thunks( \\
+        OPENSSL_sk_set_copy_thunks( \\
+            OPENSSL_sk_set_cmp_thunks( \\
+                OPENSSL_sk_dup(ossl_check_const_${nametype}_sk_type(sk)), \\
+                sk_${nametype}_cmpfunc_thunk), \\
+            sk_${nametype}_copyfunc_thunk), \\
+        sk_${nametype}_freefunc_thunk))
 #define sk_${nametype}_deep_copy(sk, copyfunc, freefunc) \\
-    ((STACK_OF(${nametype}) *)OPENSSL_sk_deep_copy( \\
-        ossl_check_const_${nametype}_sk_type(sk), \\
-        ossl_check_${nametype}_copyfunc_type(copyfunc), \\
-        ossl_check_${nametype}_freefunc_type(freefunc)))
+    ((STACK_OF(${nametype}) *)OPENSSL_sk_set_thunks( \\
+        OPENSSL_sk_set_copy_thunks( \\
+            OPENSSL_sk_set_cmp_thunks( \\
+                OPENSSL_sk_deep_copy( \\
+                    ossl_check_const_${nametype}_sk_type(sk), \\
+                    ossl_check_${nametype}_copyfunc_type(copyfunc), \\
+                    ossl_check_${nametype}_freefunc_type(freefunc)), \\
+                sk_${nametype}_cmpfunc_thunk), \\
+            sk_${nametype}_copyfunc_thunk), \\
+        sk_${nametype}_freefunc_thunk))
 #define sk_${nametype}_set_cmp_func(sk, cmp) ((sk_${nametype}_compfunc)OPENSSL_sk_set_cmp_func(ossl_check_${nametype}_sk_type(sk), ossl_check_${nametype}_compfunc_type(cmp)))
 END_MACROS