Scope reduction to enable NULL check to protect dereferencing. #3312
Conversation
Signed-off-by: sander-visser <[email protected]>
|
Hi @sander-visser and thanks for your contribution! I've reviewed and approved the change, and it will need to be approved by another team member before we can merge it. Also, it is our current policy to credit all external contributions in the ChangeLog. Could you add an entry by creating a new file in |
mpg
added
bug
component-tls
good-first-issue
Signed-off-by: sander-visser <[email protected]>
| #else | ||
| size_t out_buf_len = MBEDTLS_SSL_OUT_BUFFER_LEN; | ||
| #endif | ||
|
|
||
| mbedtls_platform_zeroize( ssl->out_buf, out_buf_len ); | ||
| mbedtls_free( ssl->out_buf ); | ||
| ssl->out_buf = NULL; |
There was a problem hiding this comment.
The whole ssl record is unconditionally zeroed at the end of the function.
This assignment could be removed.
There was a problem hiding this comment.
Indeed. And removing the assignment would also make things more consistent the other allocated structures (transform, session, etc) that we don't explicitly clear.
However IMO this is quite orthogonal to the goal of this PR, so should probably go in a separate PR.
| #else | ||
| size_t in_buf_len = MBEDTLS_SSL_IN_BUFFER_LEN; | ||
| #endif | ||
|
|
||
| mbedtls_platform_zeroize( ssl->in_buf, in_buf_len ); | ||
| mbedtls_free( ssl->in_buf ); | ||
| ssl->in_buf = NULL; |
There was a problem hiding this comment.
This assignment could be removed as well.
|
I checked the CI results, and the failure in pr-merge Mbed OS testing was an infrastructure glitch (failing to clone the repo), which is obviously unrelated to this PR and can be disregarded. |
ronald-cron-arm
added
approved
|
Could this patch enhance the bug reported in below? |
* development: (81 commits) Add changelog entry file Remove obsolete comment Changelog entry noting the behavior change and storage format change Update SE support to pass a location when registering a driver Update SE support to pass a location when registering a driver Update the SE interface to pass a location when registering a driver Fix macros Missing word Define a macro to construct a lifetime from persistence and location Document PSA_KEY_PERSISTENCE_xxx and PSA_KEY_LOCATION_xxx Rename and clarify the default persistent location and persistence PSA_KEY_LIFETIME_PERSISTENT is a lifetime, not just a storage area Shorten type and value names for lifetime parts Define some structure for lifetime values Fix typo in program benchmark. Add changelog entry for #3310. Add variable initialization to large SSL TLS function. Add Changelog entry for #3312 Scope reduction to enable NULL check to protect dereferencing. Expose SSL HW record acceleration error. ...
Signed-off-by: sander-visser [email protected]
Status
READY
Requires Backporting
NO
Migrations
NO
Additional comments
Any additional information that could be of interest
Todos
Steps to test or reproduce
Outline the steps to test or reproduce the PR here.