Fix memory leak in mbedtls by adjusting RX buffer size (issue #14444) (IDFGH-13752)

[HikingDev]

Description

This pull request addresses a memory leak issue in the mbedtls dynamic implementation related to SSL/TLS buffer management, as described in issue #14444.

Summary of Changes:

• Added rx_buffer_len() to handle correct RX buffer size
• Ensured compliance with mbedtls 16KB RX buffer requirement
• Prevents CORRUPT HEAP error when receiving large data (>16KB) over HTTPS
• Documentation Update: The esp_https_ota.rst file has been updated to provide clear guidance on mbedTLS RX buffer sizing, emphasizing the risks of reducing the RX buffer below 16KB, which can lead to memory leaks or heap corruption.
• component/mbedtls/Kconfig Update: The MBEDTLS_SSL_IN_CONTENT_LEN and MBEDTLS_DYNAMIC_BUFFER options in Kconfig were adjusted to include warnings and recommendations about proper RX buffer size management, ensuring users understand the importance of adhering to the 16KB minimum size.

Motivation: The issue arises when using the ESP32 as an HTTPS server with dynamic RX/TX buffers and receiving data larger than 16KB. The current tx_buffer_len() function in mbedtls calculates an inadequate buffer size for incoming data, leading to a heap corruption error and memory leak.

The mbedtls documentation specifies that incoming data buffers should be set to 16KB because there is no supported way to inform clients about size restrictions for incoming messages. However, the current implementation in ESP-IDF disregards this requirement. This PR ensures that the RX buffer is large enough to handle the incoming data, preventing memory corruption.

Note: While this fix ensures that the RX buffer respects the setting for CONFIG_MBEDTLS_SSL_IN_CONTENT_LEN, users should still be cautious when setting CONFIG_MBEDTLS_SSL_IN_CONTENT_LEN below 16KB, as this could still lead to memory leaks under certain conditions.

Related

• Fixes Mbedtls Dynamic Port - Memory leak (IDFGH-13555) #14444
• Includes updates to documentation (docs/en/api-reference/esp_https_ota.rst) and configuration (components/mbedtls/Kconfig) to reflect the correct handling of mbedTLS buffer sizes.

No related PRs at this time.

Testing

Tested on:

• Platform: ESP32 (v3.0) using ESP32-Wrover-Kit
• Scenario: Running an HTTPS server with dynamic TX/RX buffers.
• Results: No more memory leak or heap corruption when receiving large data (>16KB).

---

Checklist

Before submitting a Pull Request, please ensure the following:

• 🚨 This PR does not introduce breaking changes.
• All CI checks (GH Actions) pass. (apart from Sync approved PRs to internal codebase / GitHub PR to Internal Codebase Sync)
• Documentation is updated as needed.
• Tests are updated or added as necessary.
• Code is well-commented, especially in complex areas.
• Git history is clean — commits are squashed to the minimum necessary.

[github-actions]

Warnings

⚠️

Some issues found for the commit messages in this PR: the commit message "- Updated esp_https_ota.rst to clarify mbedTLS RX buffer size requirements, cautioning against setting the buffer size below 16KB due to potential memory leaks and heap corruption.": body's lines must not be longer than 100 characters summary should not end with a period (full stop) summary looks empty type/action looks empty body must have leading blank line the commit message "Fix memory leak in mbedtls by adjusting RX buffer size (issue #14444)": summary looks empty type/action looks empty Please fix these commit messages - here are some basic tips: follow Conventional Commits style correct format of commit message should be: <type/action>(<scope/component>): <summary>, for example fix(esp32): Fixed startup timeout issue allowed types are: change,ci,docs,feat,fix,refactor,remove,revert,test sufficiently descriptive message summary should be between 20 to 72 characters and start with upper case letter avoid Jira references in commit messages (unavailable/irrelevant for our customers) TIP: Install pre-commit hooks and run this check when committing (uses the Conventional Precommit Linter).

👋 Hello HikingDev, we appreciate your contribution to this project!

---

📘 Please review the project's Contributions Guide for key guidelines on code, documentation, testing, and more.

---

🖊️ Please also make sure you have read and signed the Contributor License Agreement for this project.

---

Click to see more instructions ...

This automated output is generated by the PR linter DangerJS, which checks if your Pull Request meets the project's requirements and helps you fix potential issues.

DangerJS is triggered with each push event to a Pull Request and modify the contents of this comment.

Please consider the following:
- Danger mainly focuses on the PR structure and formatting and can't understand the meaning behind your code or changes.
- Danger is not a substitute for human code reviews; it's still important to request a code review from your colleagues.
- Resolve all warnings (⚠️ ) before requesting a review from human reviewers - they will appreciate it.
- To manually retry these Danger checks, please navigate to the Actions tab and re-run last Danger workflow.

Review and merge process you can expect ...

We do welcome contributions in the form of bug reports, feature requests and pull requests via this public GitHub repository.

This GitHub project is public mirror of our internal git repository

1. An internal issue has been created for the PR, we assign it to the relevant engineer.
2. They review the PR and either approve it or ask you for changes or clarifications.
3. Once the GitHub PR is approved, we synchronize it into our internal git repository.
4. In the internal git repository we do the final review, collect approvals from core owners and make sure all the automated tests are passing.
- At this point we may do some adjustments to the proposed change, or extend it by adding tests or documentation.
5. If the change is approved and passes the tests it is merged into the default branch.
5. On next sync from the internal git repository merged change will appear in this public GitHub repository.

Generated by 🚫 dangerJS against 0109cfe

[CLAassistant]

All committers have signed the CLA.

[AdityaHPatwardhan]

Thanks for the quick PR @HikingDev,
Just to confirm does this change fix your issue of heap corruption?

[HikingDev]

Hi @AdityaHPatwardhan,

it does fix the heap corroption, if a setting of >16KB for CONFIG_MBEDTLS_SSL_IN_CONTENT_LEN is chosen.
Currently the configuration is completely disregarded and the CONFIG_MBEDTLS_SSL_OUT_CONTENT_LEN is used for both, IN and OUT Buffers.
I added a link in the Bug report to the Mbedtls docs that states, that a RX Buffer size smaller than 16KB is not supported.
Thats why I added the warnings in KConfig and Docs.
However, if someone chooses a value < 16KB for CONFIG_MBEDTLS_SSL_IN_CONTENT_LEN heap corruption can/will still happen.

Its worth considering to disallow setting MBEDTLS_SSL_IN_CONTENT_LEN at all.

config MBEDTLS_SSL_IN_CONTENT_LEN
        int "TLS maximum incoming fragment length"
        default 16384
        range 512 16384
        depends on MBEDTLS_ASYMMETRIC_CONTENT_LEN