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
Add missing range checks on number of multi primes in rsa_ossl_mod_exp #4862
Add missing range checks on number of multi primes in rsa_ossl_mod_exp #4862
Conversation
Why is this needed? I've had the time to check now, and I'm finding checks here (general RSA key checker), here (RSA key generation) and here (RSA security bits). Before |
It is at least not obvious at all why there is no buffer overrun at Line 760 in f47270e
I may be wrong, but what if a manipulated 19-key prime ASN.1 is read in, and the app does not call RSA_check_key or RSA_security_bits and goes directly to RSA_private_decrypt or RSA_sign ? |
@@ -618,7 +618,8 @@ static int rsa_ossl_mod_exp(BIGNUM *r0, const BIGNUM *I, RSA *rsa, BN_CTX *ctx) | |||
goto err; | |||
|
|||
if (rsa->version == RSA_ASN1_VERSION_MULTI | |||
&& (ex_primes = sk_RSA_PRIME_INFO_num(rsa->prime_infos)) <= 0) | |||
&& ((ex_primes = sk_RSA_PRIME_INFO_num(rsa->prime_infos)) <= 0 | |||
|| ex_primes > RSA_MAX_PRIME_NUM - 2)) |
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.
You could make that || ex_primes > sizeof(m))
...
I totally missed the intent with this PR. Sorry 'bout that, brain now awake. |
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.
Looks ok to me, regardless of if you heed my last comment or not.
Reviewed-by: Richard Levitte <levitte@openssl.org> (Merged from #4862)
Merged. Thanks! |
I think this could even crash when the imported RSA key has too many primes.