Skip to content

Fix memory leak on stream load when updating CA Certificate #6062

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

Closed
wants to merge 3 commits into from

Conversation

sguarin
Copy link

@sguarin sguarin commented Dec 25, 2021

Summary

This PR fix a memory leak when reconfiguring CA Certificate with multiple calls to WiFiClientSecure::setCACert()

Impact

When reconfiguring multiple times a CA certificate, or if your systems refresh the connection calling multiple times this function, the system will run out of memory.

Related links

#5826

@CLAassistant
Copy link

CLAassistant commented Dec 25, 2021

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@stale
Copy link

stale bot commented Apr 16, 2022

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@stale stale bot added the wontfix label Apr 16, 2022
@VojtechBartoska VojtechBartoska added Area: BT&Wifi BT & Wifi related issues and removed wontfix labels Apr 20, 2022
@github-actions
Copy link
Contributor

Unit Test Results

0 files  0 suites   0s ⏱️
0 tests 0 ✔️ 0 💤 0

Results for commit 1e962d3.

@me-no-dev me-no-dev added the Status: Needs investigation We need to do some research before taking next steps on this issue label Feb 15, 2023
@mrengineer7777
Copy link
Collaborator

@VojtechBartoska The memory leak mentioned here is resolved in PR #6387.

I agree with adding _use_insecure = false; to setCACert(), setCertificate(), setPrivateKey(). setInsecure is used in setInsecure().

@me-no-dev setInsecure() is a dangerous function as it doesn't free memory allocated in loadCACert(). But....

Calling loadCACert() after setCACert() may free a pointer that doesn't belong to this module. In my opinion loadCACert(), loadCertificate(), loadPrivateKey() should all have their own private storage variables. Then call free() for each variable in setInsecure() and ~WiFiClientSecure().

@lucasssvaz
Copy link
Collaborator

Changes already implemented.

@lucasssvaz lucasssvaz closed this Jan 29, 2024
@lucasssvaz lucasssvaz added Resolution: Duplicate Issue is a duplicate of another issue Status: Solved and removed Status: Needs investigation We need to do some research before taking next steps on this issue labels Jan 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: BT&Wifi BT & Wifi related issues Resolution: Duplicate Issue is a duplicate of another issue Status: Solved
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants