Skip to content
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

OpenSSL::PKey::PKey#sign etc. with wrong type option causes SEGV #531

Merged
merged 1 commit into from Aug 11, 2022

Conversation

no6v
Copy link
Contributor

@no6v no6v commented Aug 5, 2022

The following script causes SEGV:

require "openssl"

OpenSSL::PKey::RSA.generate(2048).sign("SHA256", "", ["x"])

I confirmed that it seems that SEGV occurs if the last optional argument (normally a Hash) finally processed by pkey_ctx_apply_options is an Array or something like that.

Since a non-nil check has done at each method function (e.g. ossl_pkey_sign) prior to call pkey_ctx_apply_options, I added Check_Type at the beginning of that function. It might be more efficient to check much earlier.

@rhenium
Copy link
Member

rhenium commented Aug 5, 2022

Thanks for the report.

Can you move the Check_Type() into pkey_ctx_apply_options0() (inside rb_protect())? pkey_ctx_apply_options() is called in a context where Ruby exceptions must not be raised directly, otherwise memory leak can occur.

@rhenium rhenium changed the base branch from master to maint-3.0 August 5, 2022 10:27
@rhenium
Copy link
Member

rhenium commented Aug 5, 2022

I changed the base branch because this needs a backport to 3.0. Please do a rebase to remove irrelevant commits in master.

causes SEGV if it is an Array or something like that.
@no6v no6v force-pushed the pkey-sign-option-type-check branch from 5584cd3 to ef23525 Compare August 8, 2022 09:52
@no6v
Copy link
Contributor Author

no6v commented Aug 8, 2022

Can you move the Check_Type() into pkey_ctx_apply_options0() (inside rb_protect())? pkey_ctx_apply_options() is called in a context where Ruby exceptions must not be raised directly, otherwise memory leak can occur.

Thanks for reviewing. I updated the patch (and the commit log a few).

@rhenium rhenium merged commit d52549a into ruby:maint-3.0 Aug 11, 2022
@rhenium
Copy link
Member

rhenium commented Aug 11, 2022

Thank you so much!

@no6v no6v deleted the pkey-sign-option-type-check branch August 13, 2022 00:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants