Skip to content

Conversation

@rhenium
Copy link
Member

@rhenium rhenium commented Feb 9, 2025

Use explicit NULL checks before interacting with STACK_OF(*). ossl_*_sk2ary() must no longer be called with NULL.

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

This PR also cleans up unreachable code involving NULL stacks:


pkcs7: add a test case for the data content type

While it is not useful alone, it is still a valid content type. Some
methods on OpenSSL::PKCS7 are only meant to work with the signed-data or
enveloped-data. Add some assertions for their behavior with unsupported
content types. The next patches will update the relevant code.


x509: do not check for negative return from X509_*_get_ext_count()

These functions are wrappers of X509v3_get_ext_count(). The
implementation can never return a negative number, and this behavior is
documented in the man page.


x509name: do not check for negative return from X509_NAME_entry_count()

The function never returns a negative number.

While it is not useful alone, it is still a valid content type. Some
methods on OpenSSL::PKCS7 are only meant to work with the signed-data
or enveloped-data content type. Add some assertions for their behavior
with unsupported content types. The next patches will update the
relevant code.
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.
These functions wrap X509v3_get_ext_count(). The implementation can
never return a negative number, and this behavior is documented in the
man page.
@rhenium rhenium force-pushed the ky/do-not-use-null-sk branch from c6446d1 to 895ce6f Compare February 11, 2025 16:30
@rhenium rhenium merged commit 3a192bb into ruby:master Feb 11, 2025
54 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant