fix: memory leak in hybrid key share generation + doc comment#10312
fix: memory leak in hybrid key share generation + doc comment#10312srpatcha wants to merge 6 commits intowolfSSL:masterfrom
Conversation
Replace ambiguous XXX placeholder in comment with descriptive text. The XXX was not a TODO marker but looked like one, causing confusion in code reviews and static analysis.
|
Can one of the admins verify this patch? |
Add explicit XFREE(ecc_kse) cleanup when the second XMALLOC for pqc_kse fails in the hybrid key share generation path. Previously, if the pqc_kse allocation failed after ecc_kse was successfully allocated, ecc_kse would be leaked since it was only tracked as a local variable.
srpatcha
changed the title
ecc_key_tmp_final was guarded by if (err == MP_OKAY), skipping cleanup on failure and leaking the temporary key. The sister function wc_ecc_mulmod_ex2 correctly calls cleanup unconditionally. Removed the conditional to match the correct pattern. Signed-off-by: Srikanth Patchava <spatchava@meta.com> Signed-off-by: Srikanth Patchava <srikanth.patchava@outlook.com>
Signed-off-by: Srikanth Patchava <spatchava@meta.com>
Signed-off-by: Srikanth Patchava <spatchava@meta.com>
Signed-off-by: Srikanth Patchava <spatchava@meta.com>
|
Hi @srpatcha , Thank you for this pull request submission. I don't see you on file as an approved contributor. We are going to review this and provide some feedback. Can you tell us more about your project , what region you are located and if you plan future submissions? Also what techniques did you use to locate these issues? Thanks, |
|
Hi @dgarske, thank you for reviewing! I'm Srikanth Patchava, a software engineer based in Sunnyvale, CA. I work on embedded systems and noticed this memory leak while studying the hybrid key share implementation. The fix ensures the allocated buffer is freed on error paths in TLSX_KeyShare_GenKey(). I'm interested in continuing to contribute to wolfSSL, particularly around memory safety and security hardening in the TLS implementation. Happy to provide any additional information needed for the contributor approval process. |
4 participants
Changes
1. Clarify session cache size comment in
ssl_sess.cDocumentation improvement to clarify the session cache size comment.
2. Fix memory leak in hybrid key share generation (
src/tls.c)In the hybrid key share generation path, if the second
XMALLOC()forpqc_ksefails afterecc_ksewas successfully allocated,ecc_kseis leaked. Added explicitXFREE(ecc_kse)cleanup in the error path.