New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix usage of custom EVP_CIPHER objects #19300
Conversation
If a custom EVP_CIPHER object has been passed to EVP_CipherInit() then it should be used in preference to a fetched cipher. We also fix a possible NULL pointer deref in the same code for digests. If the custom cipher passed to EVP_CipherInit() happens to use NID_undef (which should be a discouraged practice), then in the previous implementation this could result in the NULL cipher being fetched and hence NULL encryption being unexpectedly used. CVE-2022-3358 Fixes openssl#18970
In some circumstances we were not calling the cleanup() function to remove cipher specific data from an EVP_CIPHER_CTX.
Test that a custom EVP_CIPHER gets used in EVP_CipherInit_ex() calls.
Feedback addressed. |
Yes nice please share |
|
||
ciphctx = EVP_CIPHER_CTX_new(); | ||
if (!TEST_ptr(ciphctx) | ||
/* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the indentation off here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes it is
This pull request is ready to merge |
Needs an indentation fix before merge. |
If a custom EVP_CIPHER object has been passed to EVP_CipherInit() then it should be used in preference to a fetched cipher. We also fix a possible NULL pointer deref in the same code for digests. If the custom cipher passed to EVP_CipherInit() happens to use NID_undef (which should be a discouraged practice), then in the previous implementation this could result in the NULL cipher being fetched and hence NULL encryption being unexpectedly used. CVE-2022-3358 Fixes #18970 Reviewed-by: Tomas Mraz <tomas@openssl.org> Reviewed-by: Paul Dale <pauli@openssl.org> (Merged from #19300)
In some circumstances we were not calling the cleanup() function to remove cipher specific data from an EVP_CIPHER_CTX. Reviewed-by: Tomas Mraz <tomas@openssl.org> Reviewed-by: Paul Dale <pauli@openssl.org> (Merged from #19300)
Test that a custom EVP_CIPHER gets used in EVP_CipherInit_ex() calls. Reviewed-by: Tomas Mraz <tomas@openssl.org> Reviewed-by: Paul Dale <pauli@openssl.org> (Merged from #19300)
Pushed to master and 3.0. I fixed the indentation issue during merge. There was a trivial conflict cherry-picking to 3.0 that I also resolved during the merge. |
If a custom EVP_CIPHER object has been passed to EVP_CipherInit() then it should be used in preference to a fetched cipher. We also fix a possible NULL pointer deref in the same code for digests. If the custom cipher passed to EVP_CipherInit() happens to use NID_undef (which should be a discouraged practice), then in the previous implementation this could result in the NULL cipher being fetched and hence NULL encryption being unexpectedly used. CVE-2022-3358 Fixes #18970 Reviewed-by: Tomas Mraz <tomas@openssl.org> Reviewed-by: Paul Dale <pauli@openssl.org> (Merged from #19300) (cherry picked from commit 25d47cc)
If a custom EVP_CIPHER object has been passed to EVP_CipherInit() then it should be used in preference to a fetched cipher. We also fix a possible NULL pointer deref in the same code for digests. If the custom cipher passed to EVP_CipherInit() happens to use NID_undef (which should be a discouraged practice), then in the previous implementation this could result in the NULL cipher being fetched and hence NULL encryption being unexpectedly used. CVE-2022-3358 Fixes openssl#18970 Reviewed-by: Tomas Mraz <tomas@openssl.org> Reviewed-by: Paul Dale <pauli@openssl.org> (Merged from openssl#19300)
In some circumstances we were not calling the cleanup() function to remove cipher specific data from an EVP_CIPHER_CTX. Reviewed-by: Tomas Mraz <tomas@openssl.org> Reviewed-by: Paul Dale <pauli@openssl.org> (Merged from openssl#19300)
Test that a custom EVP_CIPHER gets used in EVP_CipherInit_ex() calls. Reviewed-by: Tomas Mraz <tomas@openssl.org> Reviewed-by: Paul Dale <pauli@openssl.org> (Merged from openssl#19300)
If a custom EVP_CIPHER object has been passed to EVP_CipherInit() then it
should be used in preference to a fetched cipher.
We also fix a possible NULL pointer deref in the same code for digests.
If the custom cipher passed to EVP_CipherInit() happens to use NID_undef
(which should be a discouraged practice), then in the previous
implementation this could result in the NULL cipher being fetched and
hence NULL encryption being unexpectedly used.
CVE-2022-3358
Fixes #18970
Note that this has been assessed as a low severity security issue. As per our policy, since it is low, it is being fixed in public and will be included in the next release.
We also fix a bug where the cleanup function for an EVP_CIPHER was not always being called when it should, which was highlighted by writing the test for the original issue.