[ruby/openssl] ossl.c: avoid using sk_*() functions with NULL

Always use explicit NULL checks before interacting with STACK_OF(*).
Even though most OpenSSL functions named sk_*() do not crash if we pass
NULL as the receiver object, depending on this behavior would be a bad
idea.

Checks for a negative number return from sk_*_num() are removed. This
can only happen when the stack is NULL.

ossl_*_sk2ary() must no longer be called with NULL.

https://github.com/ruby/openssl/commit/84cffd4f77
This commit is contained in:
Kazuki Yamaguchi 2025-01-07 02:14:46 +09:00 committed by git
parent dedd05e9c8
commit 8888ad6902
4 changed files with 40 additions and 43 deletions

View File

@ -69,16 +69,9 @@ ossl_##name##_sk2ary(const STACK_OF(type) *sk) \
int i, num; \
VALUE ary; \
\
if (!sk) { \
OSSL_Debug("empty sk!"); \
return Qnil; \
} \
RUBY_ASSERT(sk != NULL); \
num = sk_##type##_num(sk); \
if (num < 0) { \
OSSL_Debug("items in sk < -1???"); \
return rb_ary_new(); \
} \
ary = rb_ary_new2(num); \
ary = rb_ary_new_capa(num); \
\
for (i=0; i<num; i++) { \
t = sk_##type##_value(sk, i); \

View File

@ -557,21 +557,16 @@ ossl_pkcs7_get_signer(VALUE self)
{
PKCS7 *pkcs7;
STACK_OF(PKCS7_SIGNER_INFO) *sk;
PKCS7_SIGNER_INFO *si;
int num, i;
VALUE ary;
GetPKCS7(self, pkcs7);
if (!(sk = PKCS7_get_signer_info(pkcs7))) {
OSSL_Debug("OpenSSL::PKCS7#get_signer_info == NULL!");
return rb_ary_new();
}
if ((num = sk_PKCS7_SIGNER_INFO_num(sk)) < 0) {
ossl_raise(ePKCS7Error, "Negative number of signers!");
}
ary = rb_ary_new2(num);
if (!(sk = PKCS7_get_signer_info(pkcs7)))
return rb_ary_new();
num = sk_PKCS7_SIGNER_INFO_num(sk);
ary = rb_ary_new_capa(num);
for (i=0; i<num; i++) {
si = sk_PKCS7_SIGNER_INFO_value(sk, i);
PKCS7_SIGNER_INFO *si = sk_PKCS7_SIGNER_INFO_value(sk, i);
rb_ary_push(ary, ossl_pkcs7si_new(si));
}
@ -604,7 +599,6 @@ ossl_pkcs7_get_recipient(VALUE self)
{
PKCS7 *pkcs7;
STACK_OF(PKCS7_RECIP_INFO) *sk;
PKCS7_RECIP_INFO *si;
int num, i;
VALUE ary;
@ -615,13 +609,11 @@ ossl_pkcs7_get_recipient(VALUE self)
sk = pkcs7->d.signed_and_enveloped->recipientinfo;
else sk = NULL;
if (!sk) return rb_ary_new();
if ((num = sk_PKCS7_RECIP_INFO_num(sk)) < 0) {
ossl_raise(ePKCS7Error, "Negative number of recipient!");
}
ary = rb_ary_new2(num);
num = sk_PKCS7_RECIP_INFO_num(sk);
ary = rb_ary_new_capa(num);
for (i=0; i<num; i++) {
si = sk_PKCS7_RECIP_INFO_value(sk, i);
rb_ary_push(ary, ossl_pkcs7ri_new(si));
PKCS7_RECIP_INFO *ri = sk_PKCS7_RECIP_INFO_value(sk, i);
rb_ary_push(ary, ossl_pkcs7ri_new(ri));
}
return ary;
@ -701,7 +693,10 @@ ossl_pkcs7_set_certificates(VALUE self, VALUE ary)
X509 *cert;
certs = pkcs7_get_certs(self);
while((cert = sk_X509_pop(certs))) X509_free(cert);
if (certs) {
while ((cert = sk_X509_pop(certs)))
X509_free(cert);
}
rb_block_call(ary, rb_intern("each"), 0, 0, ossl_pkcs7_set_certs_i, self);
return ary;
@ -710,7 +705,10 @@ ossl_pkcs7_set_certificates(VALUE self, VALUE ary)
static VALUE
ossl_pkcs7_get_certificates(VALUE self)
{
return ossl_x509_sk2ary(pkcs7_get_certs(self));
STACK_OF(X509) *certs = pkcs7_get_certs(self);
if (!certs)
return Qnil;
return ossl_x509_sk2ary(certs);
}
static VALUE
@ -741,7 +739,10 @@ ossl_pkcs7_set_crls(VALUE self, VALUE ary)
X509_CRL *crl;
crls = pkcs7_get_crls(self);
while((crl = sk_X509_CRL_pop(crls))) X509_CRL_free(crl);
if (crls) {
while ((crl = sk_X509_CRL_pop(crls)))
X509_CRL_free(crl);
}
rb_block_call(ary, rb_intern("each"), 0, 0, ossl_pkcs7_set_crls_i, self);
return ary;
@ -750,7 +751,10 @@ ossl_pkcs7_set_crls(VALUE self, VALUE ary)
static VALUE
ossl_pkcs7_get_crls(VALUE self)
{
return ossl_x509crl_sk2ary(pkcs7_get_crls(self));
STACK_OF(X509_CRL) *crls = pkcs7_get_crls(self);
if (!crls)
return Qnil;
return ossl_x509crl_sk2ary(crls);
}
static VALUE

View File

@ -2450,7 +2450,7 @@ ossl_ssl_get_peer_finished(VALUE self)
/*
* call-seq:
* ssl.client_ca => [x509name, ...]
* ssl.client_ca => [x509name, ...] or nil
*
* Returns the list of client CAs. Please note that in contrast to
* SSLContext#client_ca= no array of X509::Certificate is returned but
@ -2468,6 +2468,8 @@ ossl_ssl_get_client_ca_list(VALUE self)
GetSSL(self, ssl);
ca = SSL_get_client_CA_list(ssl);
if (!ca)
return Qnil;
return ossl_x509name_sk2ary(ca);
}

View File

@ -276,21 +276,19 @@ ossl_x509crl_get_revoked(VALUE self)
{
X509_CRL *crl;
int i, num;
X509_REVOKED *rev;
VALUE ary, revoked;
STACK_OF(X509_REVOKED) *sk;
VALUE ary;
GetX509CRL(self, crl);
num = sk_X509_REVOKED_num(X509_CRL_get_REVOKED(crl));
if (num < 0) {
OSSL_Debug("num < 0???");
return rb_ary_new();
}
ary = rb_ary_new2(num);
sk = X509_CRL_get_REVOKED(crl);
if (!sk)
return rb_ary_new();
num = sk_X509_REVOKED_num(sk);
ary = rb_ary_new_capa(num);
for(i=0; i<num; i++) {
/* NO DUP - don't free! */
rev = sk_X509_REVOKED_value(X509_CRL_get_REVOKED(crl), i);
revoked = ossl_x509revoked_new(rev);
rb_ary_push(ary, revoked);
X509_REVOKED *rev = sk_X509_REVOKED_value(sk, i);
rb_ary_push(ary, ossl_x509revoked_new(rev));
}
return ary;