Skip to content

Commit 81df150

Browse files
committed
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.
1 parent 5ccaccd commit 81df150

1 file changed

Lines changed: 11 additions & 7 deletions

File tree

ext/openssl/openssl_backend_common.c

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -691,6 +691,7 @@ STACK_OF(X509) *php_openssl_load_all_certs_from_file(
691691
BIO *in=NULL;
692692
X509_INFO *xi;
693693
char cert_path[MAXPATHLEN];
694+
int i;
694695

695696
if (!php_openssl_check_path(cert_file, cert_file_len, cert_path, arg_num)) {
696697
goto end;
@@ -709,29 +710,32 @@ STACK_OF(X509) *php_openssl_load_all_certs_from_file(
709710
goto end;
710711
}
711712

712-
if(!(stack = sk_X509_new_reserve(NULL, sk_X509_INFO_num(sk)))) {
713+
if(!(stack = sk_X509_new_null())) {
713714
php_openssl_store_errors();
714715
goto end;
715716
}
716717

717718
/* scan over it and pull out the certs */
718-
while (sk_X509_INFO_num(sk)) {
719-
xi=sk_X509_INFO_shift(sk);
719+
for (i = 0; i < sk_X509_INFO_num(sk); i++) {
720+
xi=sk_X509_INFO_value(sk,i);
720721
if (xi->x509 != NULL) {
721-
sk_X509_push(stack,xi->x509);
722+
if (!sk_X509_push(stack,xi->x509)) {
723+
php_error_docref(NULL, E_ERROR, "Memory allocation failure");
724+
goto end;
725+
}
722726
xi->x509=NULL;
723727
}
724-
X509_INFO_free(xi);
725728
}
726729
if (!sk_X509_num(stack)) {
727730
php_error_docref(NULL, E_WARNING, "No certificates in file, %s", cert_path);
728-
sk_X509_free(stack);
729731
goto end;
730732
}
731733
ret = stack;
734+
stack = NULL;
732735
end:
733736
BIO_free(in);
734-
sk_X509_INFO_free(sk);
737+
sk_X509_INFO_pop_free(sk,X509_INFO_free);
738+
sk_X509_pop_free(stack,X509_free);
735739

736740
return ret;
737741
}

0 commit comments

Comments
 (0)