Limit size of modulus for BN_mod_exp_mont_consttime()#19632
Limit size of modulus for BN_mod_exp_mont_consttime()#19632t8m wants to merge 1 commit intoopenssl:masterfrom
Conversation
t8m
added
branch: master
|
Although not a security issue it is kind of hardening so we might want to consider this for 1.1.1 as well. |
Otherwise the powerbufLen can overflow. Issue reported by Jiayi Lin.
|
|
||
| if (!TEST_true(a_is_zero_mod_one("BN_mod_exp_mont_consttime", r, a))) | ||
| failed = 1; | ||
|
|
There was a problem hiding this comment.
I guess this test doesn't really cover the overflow condition....but I'm not sure how it is even possible to reasonably test that???
There was a problem hiding this comment.
Yeah. IMO it is not possible, the test would require too much memory to run, i.e. it could fail randomly if run on machines with small memory and it could also possibly be a CPU hog.
There was a problem hiding this comment.
The added test case tests just the BN_MONT_CTX not set up.
beldmit
added
approval: done
|
24 hours has passed since 'approval: done' was set, but this PR has failing CI tests. Once the tests pass it will get moved to 'approval: ready to merge' automatically, alternatively please review and set the label manually. |
Otherwise the powerbufLen can overflow. Issue reported by Jiayi Lin. Reviewed-by: Matt Caswell <[email protected]> Reviewed-by: Dmitry Belyavskiy <[email protected]> (Merged from #19632) (cherry picked from commit 4378e3c)
Otherwise the powerbufLen can overflow. Issue reported by Jiayi Lin. Reviewed-by: Matt Caswell <[email protected]> Reviewed-by: Dmitry Belyavskiy <[email protected]> (Merged from #19632) (cherry picked from commit 4378e3c)
Otherwise the powerbufLen can overflow. Issue reported by Jiayi Lin. Reviewed-by: Matt Caswell <[email protected]> Reviewed-by: Dmitry Belyavskiy <[email protected]> (Merged from #19632)
|
Merged to master, 3.0, and 3.1 branches. Thank you for the reviews. 1.1.1 needs tweaks so created a separate PR. |
Otherwise the powerbufLen can overflow. Issue reported by Jiayi Lin. Reviewed-by: Matt Caswell <[email protected]> Reviewed-by: Dmitry Belyavskiy <[email protected]> (Merged from openssl#19632)
5 participants
Otherwise the powerbufLen can overflow.
Issue reported by Jiayi Lin.