From: Nikola Pajkovsky Date: Wed, 20 May 2026 06:51:07 +0000 (+0200) Subject: preserve stack thunks across sk_TYPE_dup X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=a1e4bd14e59e62943fe048b944eac894cd0d8489;p=thirdparty%2Fopenssl.git 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 Reviewed-by: Neil Horman Reviewed-by: Milan Broz 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 a4c178d1897..64ac28be049 100644 --- 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 848c843931f..16d72f809e4 100644 --- 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 e49e2d4defe..b3446f77478 100644 --- 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