Skip to content

Fix WiFiClientSecure memory leaks when the connection fails (certificate verification, handshake, etc.) #5944

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

ppescher
Copy link
Contributor

This should fix issue #5781 and possibly others related to the same problem: the function start_ssl_client() does not free memory allocated by mbed-tls library for data structures holding CA certificate, client certificate and private key, in case of early failures, but only when the connection is successful.

The proposed fix adds some cleanup code to stop_ssl_socket() so that WiFiSecureClient does not leak memory when it finally calls stop().

ppescher and others added 3 commits May 19, 2021 19:06
Thi may happen if read() gets called repeatedly (such as in HttpClient to parse response headers) and the connection is closed unexpectedly or the remote peer may have unexpected behavior that causes the underlying socket to report an error. In that case read() itself calls stop(), which invalidates the receive buffer object. Then when read() is called again without checking, such as inside readStringUntil(), the _rxBuffer is null and ESP32 crashes.
TLS data structures for CA certificate chain and client certifcate and private key were not deallocated if the function returns earlier due to some error. The fix makes sure to free this data in addition to the other cleanup already present in stop_ssl_socket(), which is called by the client when start_ssl_client() fails.
@ppescher ppescher closed this Nov 29, 2021
@ppescher ppescher deleted the fix-ssl-client-memleak branch November 29, 2021 13:34
@ppescher
Copy link
Contributor Author

Closed because of wrong patch, repo was not in-sync with origin.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant