esp-aws-iot icon indicating copy to clipboard operation
esp-aws-iot copied to clipboard

Possible memory leak in ota_pal.c (CA-296)

Open gonzzor opened this issue 1 year ago • 2 comments

In the function otaPal_CheckFileSignature() there are two possible code paths that lead to memory leaks.

The call to CRYPTO_SignatureVerificationStart will allocate memory but it's not freed until CRYPTO_SignatureVerificationFinal() is called.

Between these two calls there are

  • One return statement if the certificate wasn't properly set, https://github.com/espressif/esp-aws-iot/blob/1fc7681778bc271960a4e3db514a209df0380917/libraries/ota-for-aws-iot-embedded-sdk/port/ota_pal.c#L425
  • One goto that skip the proper code path. https://github.com/espressif/esp-aws-iot/blob/1fc7681778bc271960a4e3db514a209df0380917/libraries/ota-for-aws-iot-embedded-sdk/port/ota_pal.c#L451

Both these cases will cause a memory leak. Both code paths are rather unlikely but they still exist and shouldn't leak memory.

gonzzor avatar Jun 06 '23 17:06 gonzzor

https://github.com/espressif/esp-aws-iot/commit/24c17abd6f51d70531f3eaa0e287c7397da80582 is meant to fix this, but there's two new cases that will fail.

Both if the verification fails or succeeds here, it will free the memory block in CRYPTO_SignatureVerificationFinal and it will then try to free that memory block again causing the esp32 to crash. https://github.com/espressif/esp-aws-iot/blob/24c17abd6f51d70531f3eaa0e287c7397da80582/libraries/ota-for-aws-iot-embedded-sdk/port/ota_pal.c#L468-L478 Line 472 and 477 should return. @avsheth

EDIT: I assume the case where pvContext == NULL in CRYPTO_SignatureVerificationFinal is ok not to free the memory.

AntoineSX avatar Oct 03 '23 15:10 AntoineSX

Hi @AntoineSX Yeah, guess I made a blunder :) Thanks for bringing it to the notice. Will push the fix.

avsheth avatar Oct 03 '23 16:10 avsheth