mbedtls icon indicating copy to clipboard operation
mbedtls copied to clipboard

Hardcoded, insufficient hash buffer size

Open leorosen opened this issue 2 years ago • 2 comments

Summary

In several code locations, a has result buffer is declared with a hard-coded size of 48. This does not account for the fact that SHA512 and SHA384 are overlaid, and by enabling one, the other is also enabled. If for any reason SHA512 will be activated, those buffers will be written beyond boundary.

System information

Mbed TLS version (v3,1,2): Operating system and version: bare metal Configuration (if not default, please attach mbedtls_config.h): n/a Compiler and options (if you used a pre-built binary, please indicate how you obtained it): vendor Additional environment information:

Expected behavior

Actual behavior

Steps to reproduce

Activate the already allowed SHA512 hash

Additional information

The issue was detected by Coverity static code analysis.

leorosen avatar Jul 28 '22 22:07 leorosen

@leorosen Thank you for your report and for submitting the fix.

I am not sure I can see the potential for buffer overflow here. I have commented on your PR with the details.

yanesca avatar Aug 01 '22 10:08 yanesca

Please see these two branches which during test fail on assertion, proving that the suspected buffer overflow happens: https://github.com/leorosen/mbedtls/tree/leorosen-stack-watch-ssl_tls12_client

Test moving clients handshake to state: CERTIFICATE_REQUEST ....... Assertion failed: (x == 0), function ssl_parse_server_key_exchange, file ssl_tls12_client.c, line 2475.

https://github.com/leorosen/mbedtls/tree/leorosen-stack-watch-ssl_tls12_server

Test moving clients handshake to state: CERTIFICATE_REQUEST ....... Assertion failed: (x == 0), function ssl_prepare_server_key_exchange, file ssl_tls12_server.c, line 3114.

leorosen avatar Aug 03 '22 18:08 leorosen

I still can't see a buffer overflow here: https://github.com/Mbed-TLS/mbedtls/pull/6153#issuecomment-1204507139

If you think that it is still there, please post information for reproducing it and we'll reopen when we managed to reproduce.

yanesca avatar Aug 18 '22 07:08 yanesca