From 81df150bc3d3c302dc78a81c9879f8fa176601bb Mon Sep 17 00:00:00 2001 From: Theo Buehler Date: Fri, 13 Mar 2026 04:24:14 +0100 Subject: [PATCH] Simplify php_openssl_load_all_certs_from_file() Correctly free both stacks, sk and stack, in the exit path and transfer ownership of stack to ret on success. Don't free the members of sk in the loop, only steal the certs after successful sk_X509_push(), which is error checked as it always should be. Switch from sk_new_reserve() back to the much saner sk_X509_new_null(). This makes ownership in this code easier to reason about and more robust to future modifications. --- ext/openssl/openssl_backend_common.c | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/ext/openssl/openssl_backend_common.c b/ext/openssl/openssl_backend_common.c index 6f257af49819..18e4bca0736e 100644 --- a/ext/openssl/openssl_backend_common.c +++ b/ext/openssl/openssl_backend_common.c @@ -691,6 +691,7 @@ STACK_OF(X509) *php_openssl_load_all_certs_from_file( BIO *in=NULL; X509_INFO *xi; char cert_path[MAXPATHLEN]; + int i; if (!php_openssl_check_path(cert_file, cert_file_len, cert_path, arg_num)) { goto end; @@ -709,29 +710,32 @@ STACK_OF(X509) *php_openssl_load_all_certs_from_file( goto end; } - if(!(stack = sk_X509_new_reserve(NULL, sk_X509_INFO_num(sk)))) { + if(!(stack = sk_X509_new_null())) { php_openssl_store_errors(); goto end; } /* scan over it and pull out the certs */ - while (sk_X509_INFO_num(sk)) { - xi=sk_X509_INFO_shift(sk); + for (i = 0; i < sk_X509_INFO_num(sk); i++) { + xi=sk_X509_INFO_value(sk,i); if (xi->x509 != NULL) { - sk_X509_push(stack,xi->x509); + if (!sk_X509_push(stack,xi->x509)) { + php_error_docref(NULL, E_ERROR, "Memory allocation failure"); + goto end; + } xi->x509=NULL; } - X509_INFO_free(xi); } if (!sk_X509_num(stack)) { php_error_docref(NULL, E_WARNING, "No certificates in file, %s", cert_path); - sk_X509_free(stack); goto end; } ret = stack; + stack = NULL; end: BIO_free(in); - sk_X509_INFO_free(sk); + sk_X509_INFO_pop_free(sk,X509_INFO_free); + sk_X509_pop_free(stack,X509_free); return ret; }