Zero AllocKey's allocation so its failure path cannot free garbage#10873
Closed
PhilipTaronQ wants to merge 1 commit into
Closed
Zero AllocKey's allocation so its failure path cannot free garbage#10873PhilipTaronQ wants to merge 1 commit into
PhilipTaronQ wants to merge 1 commit into
Conversation
AllocKey hands FreeKey a never-initialized key whenever the type's init function fails: the allocation comes straight from XMALLOC, and e.g. wc_FreeRsaKey then walks uninitialized multiprecision integers whose length fields are garbage. AddressSanitizer caught sp_clear doing a 390KB memset past the allocation during a TLS 1.3 handshake whose wc_InitRsaKey failed transiently (a FIPS known-answer-test error), so a recoverable handshake error becomes heap corruption. Zero the key after allocation; every wc_Free* implementation treats an all-zero key as empty. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Can one of the admins verify this patch? |
Member
|
ZD 22133 |
Member
|
Hi @PhilipTaronQ , thank you for this suggested fix. For items that are small like this its easier to support a bug report by emailing support@wolfssl.com or adding to your existing zendesk ticket 22133. Also we can't accept commits with co-author of AI. Thanks, David Garske, wolfSSL |
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
3 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.
Description
AllocKeyhandsFreeKeya never-initialized key whenever the type's init function fails: the allocation comes straight fromXMALLOC, and e.g.wc_FreeRsaKeythen walks uninitialized multiprecision integers whose length fields are garbage. AddressSanitizer caught sp_clear doing a 390KB memset past the allocation during a TLS 1.3 handshake whosewc_InitRsaKeyfailed transiently (a FIPS known-answer-test error), so a recoverable handshake error becomes... heap corruption.Zero the key after allocation; every
wc_Free*implementation treats an all-zero key as empty.Testing
I built the Qumulo product with this patch; prior to that, we had sporadic ASAN failures.
Checklist