diff options
| author | tb <> | 2023-12-14 12:02:10 +0000 |
|---|---|---|
| committer | tb <> | 2023-12-14 12:02:10 +0000 |
| commit | 12a7c59c2985d4787deb1a503da3768c45dd7b22 (patch) | |
| tree | 4908d2d7ad0072f6f41bdbb32a1e687f4e61d0a5 | |
| parent | 1d1bc4a77d9495d0441867034632ed5ba3730a44 (diff) | |
| download | openbsd-12a7c59c2985d4787deb1a503da3768c45dd7b22.tar.gz openbsd-12a7c59c2985d4787deb1a503da3768c45dd7b22.tar.bz2 openbsd-12a7c59c2985d4787deb1a503da3768c45dd7b22.zip | |
Fix sk_deep_copy() implementation
sk_deep_copy() is bad code. It is less bad than the upstream code, but
still bad: it passes strdup() through a void pointer and assigns it to
a function pointer of different type before calling the latter. That's
not kosher in more than one way.
There is no need for such gymnastics. If we need a deep copy for a type,
we should implement it as appropriate for that type.
Also, we should not expect and even less so allow holes in a STACK_OF().
The only way the vpm->hosts can be populated is by way of this deep_copy
function or x509_param_set_hosts_internal(), which pushes only after a
non-NULL check. Invariants: they're useful.
ok jsing
| -rw-r--r-- | src/lib/libcrypto/x509/x509_vpm.c | 54 |
1 files changed, 22 insertions, 32 deletions
diff --git a/src/lib/libcrypto/x509/x509_vpm.c b/src/lib/libcrypto/x509/x509_vpm.c index 4ba697ead4..662e3179a6 100644 --- a/src/lib/libcrypto/x509/x509_vpm.c +++ b/src/lib/libcrypto/x509/x509_vpm.c | |||
| @@ -1,4 +1,4 @@ | |||
| 1 | /* $OpenBSD: x509_vpm.c,v 1.40 2023/05/28 05:25:24 tb Exp $ */ | 1 | /* $OpenBSD: x509_vpm.c,v 1.41 2023/12/14 12:02:10 tb Exp $ */ |
| 2 | /* Written by Dr Stephen N Henson (steve@openssl.org) for the OpenSSL | 2 | /* Written by Dr Stephen N Henson (steve@openssl.org) for the OpenSSL |
| 3 | * project 2004. | 3 | * project 2004. |
| 4 | */ | 4 | */ |
| @@ -84,41 +84,31 @@ str_free(char *s) | |||
| 84 | free(s); | 84 | free(s); |
| 85 | } | 85 | } |
| 86 | 86 | ||
| 87 | /* | 87 | static STACK_OF(OPENSSL_STRING) * |
| 88 | * Post 1.0.1 sk function "deep_copy". For the moment we simply make | 88 | sk_OPENSSL_STRING_deep_copy(const STACK_OF(OPENSSL_STRING) *sk) |
| 89 | * these take void * and use them directly without a glorious blob of | ||
| 90 | * obfuscating macros of dubious value in front of them. All this in | ||
| 91 | * preparation for a rototilling of safestack.h (likely inspired by | ||
| 92 | * this). | ||
| 93 | */ | ||
| 94 | static void * | ||
| 95 | sk_deep_copy(void *sk_void, void *copy_func_void, void *free_func_void) | ||
| 96 | { | 89 | { |
| 97 | _STACK *sk = sk_void; | 90 | STACK_OF(OPENSSL_STRING) *new; |
| 98 | void *(*copy_func)(void *) = copy_func_void; | 91 | char *copy = NULL; |
| 99 | void (*free_func)(void *) = free_func_void; | 92 | int i; |
| 100 | _STACK *ret = sk_dup(sk); | ||
| 101 | size_t i; | ||
| 102 | 93 | ||
| 103 | if (ret == NULL) | 94 | if ((new = sk_OPENSSL_STRING_new_null()) == NULL) |
| 104 | return NULL; | 95 | goto err; |
| 105 | 96 | ||
| 106 | for (i = 0; i < ret->num; i++) { | 97 | for (i = 0; i < sk_OPENSSL_STRING_num(sk); i++) { |
| 107 | if (ret->data[i] == NULL) | 98 | if ((copy = strdup(sk_OPENSSL_STRING_value(sk, i))) == NULL) |
| 108 | continue; | 99 | goto err; |
| 109 | ret->data[i] = copy_func(ret->data[i]); | 100 | if (sk_OPENSSL_STRING_push(new, copy) <= 0) |
| 110 | if (ret->data[i] == NULL) { | 101 | goto err; |
| 111 | size_t j; | 102 | copy = NULL; |
| 112 | for (j = 0; j < i; j++) { | ||
| 113 | if (ret->data[j] != NULL) | ||
| 114 | free_func(ret->data[j]); | ||
| 115 | } | ||
| 116 | sk_free(ret); | ||
| 117 | return NULL; | ||
| 118 | } | ||
| 119 | } | 103 | } |
| 120 | 104 | ||
| 121 | return ret; | 105 | return new; |
| 106 | |||
| 107 | err: | ||
| 108 | sk_OPENSSL_STRING_pop_free(new, str_free); | ||
| 109 | free(copy); | ||
| 110 | |||
| 111 | return NULL; | ||
| 122 | } | 112 | } |
| 123 | 113 | ||
| 124 | static int | 114 | static int |
| @@ -313,7 +303,7 @@ X509_VERIFY_PARAM_inherit(X509_VERIFY_PARAM *dest, const X509_VERIFY_PARAM *src) | |||
| 313 | dest->hosts = NULL; | 303 | dest->hosts = NULL; |
| 314 | } | 304 | } |
| 315 | if (src->hosts) { | 305 | if (src->hosts) { |
| 316 | dest->hosts = sk_deep_copy(src->hosts, strdup, str_free); | 306 | dest->hosts = sk_OPENSSL_STRING_deep_copy(src->hosts); |
| 317 | if (dest->hosts == NULL) | 307 | if (dest->hosts == NULL) |
| 318 | return 0; | 308 | return 0; |
| 319 | } | 309 | } |