Prevent ECC tmp key leak and UB#10346
Open
Frauschi wants to merge 1 commit intowolfSSL:masterfrom
Open
Conversation
ecc_key_tmp_final was guarded by `if (err == MP_OKAY)`, leaking key->t1/t2 (and x/y/z under ALT_ECC_SIZE) whenever an allocation or mulmod step after ecc_key_tmp_init failed. Simply removing the guard is unsafe here: unlike wc_ecc_mulmod_ex2 (whose arg checks `return` directly), this function XMALLOC'd `key` before the arg checks and used `goto exit`, so a bad-arg call would hand uninitialized memory to ecc_key_tmp_final and XFREE garbage pointers. Defer the XMALLOC until after the arg/range checks so `key` is NULL on the early-error paths, then call ecc_key_tmp_final unconditionally to plug the leak on the late-error paths.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ecc_key_tmp_finalwas guarded byif (err == MP_OKAY), leakingkey->t1/t2(andx/y/zunderALT_ECC_SIZE) whenever an allocation or mulmod step afterecc_key_tmp_initfailed.Simply removing the guard is unsafe however: unlike
wc_ecc_mulmod_ex2(whose arg checksreturndirectly), this functionXMALLOC'dkeybefore the arg checks and usedgoto exit, so a bad-arg call would hand uninitialized memory toecc_key_tmp_finalandXFREEgarbage pointers.Defer the
XMALLOCuntil after the arg/range checks sokeyisNULLon the early-error paths, then callecc_key_tmp_finalunconditionally to plug the leak on the late-error paths.Identified by Srikanth Patchava srikanth.patchava@outlook.com