From 358f94a71c81bdaf95c5cab0e5ce79c541bf2cf9 Mon Sep 17 00:00:00 2001 From: Ashley Duncan Date: Fri, 11 Feb 2022 09:57:18 +1300 Subject: [PATCH 1/6] Fixed undefined behavior in ssl_read if buf parameter is NULL. Signed-off-by: Ashley Duncan --- library/ssl_msg.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/library/ssl_msg.c b/library/ssl_msg.c index d26d95086..1162cca02 100644 --- a/library/ssl_msg.c +++ b/library/ssl_msg.c @@ -5593,8 +5593,10 @@ int mbedtls_ssl_read(mbedtls_ssl_context *ssl, unsigned char *buf, size_t len) n = (len < ssl->in_msglen) ? len : ssl->in_msglen; - memcpy(buf, ssl->in_offt, n); - ssl->in_msglen -= n; + if (buf) { + memcpy(buf, ssl->in_offt, n); + ssl->in_msglen -= n; + } /* Zeroising the plaintext buffer to erase unused application data from the memory. */ From 937d6d5eab6b28769cae2c8e5fbd41c94c9919ee Mon Sep 17 00:00:00 2001 From: ashesman Date: Thu, 17 Feb 2022 11:08:27 +1300 Subject: [PATCH 2/6] Update library/ssl_msg.c Co-authored-by: Gilles Peskine Signed-off-by: Dave Rodgman --- library/ssl_msg.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/ssl_msg.c b/library/ssl_msg.c index 1162cca02..5deaedacc 100644 --- a/library/ssl_msg.c +++ b/library/ssl_msg.c @@ -5593,7 +5593,7 @@ int mbedtls_ssl_read(mbedtls_ssl_context *ssl, unsigned char *buf, size_t len) n = (len < ssl->in_msglen) ? len : ssl->in_msglen; - if (buf) { + if (len != 0) { memcpy(buf, ssl->in_offt, n); ssl->in_msglen -= n; } From 88240e769f974f506f4f5f78fa6acd66d7ae4447 Mon Sep 17 00:00:00 2001 From: Ashley Duncan Date: Thu, 17 Feb 2022 11:10:33 +1300 Subject: [PATCH 3/6] Added changelog entry. Signed-off-by: Ashley Duncan --- ChangeLog.d/mbedtls_ssl_read_undefined_behavior.txt | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 ChangeLog.d/mbedtls_ssl_read_undefined_behavior.txt diff --git a/ChangeLog.d/mbedtls_ssl_read_undefined_behavior.txt b/ChangeLog.d/mbedtls_ssl_read_undefined_behavior.txt new file mode 100644 index 000000000..392a91b72 --- /dev/null +++ b/ChangeLog.d/mbedtls_ssl_read_undefined_behavior.txt @@ -0,0 +1,2 @@ +Bugfix + * Fixed undefined behavior in mbedtls_ssl_read if len argument is 0 From f68402565ab0d52e556c0bbfa89feec292133cb9 Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Fri, 24 Feb 2023 15:41:34 +0000 Subject: [PATCH 4/6] Add corresponding fix for mbedtls_ssl_write Signed-off-by: Dave Rodgman --- library/ssl_msg.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/library/ssl_msg.c b/library/ssl_msg.c index 5deaedacc..4ed67b707 100644 --- a/library/ssl_msg.c +++ b/library/ssl_msg.c @@ -5672,7 +5672,9 @@ static int ssl_write_real(mbedtls_ssl_context *ssl, */ ssl->out_msglen = len; ssl->out_msgtype = MBEDTLS_SSL_MSG_APPLICATION_DATA; - memcpy(ssl->out_msg, buf, len); + if (len > 0) { + memcpy(ssl->out_msg, buf, len); + } if ((ret = mbedtls_ssl_write_record(ssl, SSL_FORCE_FLUSH)) != 0) { MBEDTLS_SSL_DEBUG_RET(1, "mbedtls_ssl_write_record", ret); From fd8929cfd112b8c40f9d48c02fa3658520767bd1 Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Fri, 24 Feb 2023 15:43:43 +0000 Subject: [PATCH 5/6] Improve changelog Signed-off-by: Dave Rodgman --- ChangeLog.d/mbedtls_ssl_read_undefined_behavior.txt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ChangeLog.d/mbedtls_ssl_read_undefined_behavior.txt b/ChangeLog.d/mbedtls_ssl_read_undefined_behavior.txt index 392a91b72..1f2c563be 100644 --- a/ChangeLog.d/mbedtls_ssl_read_undefined_behavior.txt +++ b/ChangeLog.d/mbedtls_ssl_read_undefined_behavior.txt @@ -1,2 +1,3 @@ Bugfix - * Fixed undefined behavior in mbedtls_ssl_read if len argument is 0 + * Fix undefined behavior in mbedtls_ssl_read() and mbedtls_ssl_write() if + len argument is 0 and buffer is NULL. From a4e8fb0041191e75970cb2e2b1ea70b915b87afb Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Fri, 24 Feb 2023 15:41:55 +0000 Subject: [PATCH 6/6] Add tests Signed-off-by: Dave Rodgman --- tests/suites/test_suite_ssl.function | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/suites/test_suite_ssl.function b/tests/suites/test_suite_ssl.function index 15246cb1a..f92048596 100644 --- a/tests/suites/test_suite_ssl.function +++ b/tests/suites/test_suite_ssl.function @@ -1159,6 +1159,12 @@ int mbedtls_ssl_write_fragment(mbedtls_ssl_context *ssl, unsigned char *buf, int buf_len, int *written, const int expected_fragments) { + /* Verify that calling mbedtls_ssl_write with a NULL buffer and zero length is + * a valid no-op for TLS connections. */ + if (ssl->conf->transport != MBEDTLS_SSL_TRANSPORT_DATAGRAM) { + TEST_ASSERT(mbedtls_ssl_write(ssl, NULL, 0) == 0); + } + int ret = mbedtls_ssl_write(ssl, buf + *written, buf_len - *written); if (ret > 0) { *written += ret; @@ -1197,6 +1203,12 @@ int mbedtls_ssl_read_fragment(mbedtls_ssl_context *ssl, unsigned char *buf, int buf_len, int *read, int *fragments, const int expected_fragments) { + /* Verify that calling mbedtls_ssl_write with a NULL buffer and zero length is + * a valid no-op for TLS connections. */ + if (ssl->conf->transport != MBEDTLS_SSL_TRANSPORT_DATAGRAM) { + TEST_ASSERT(mbedtls_ssl_read(ssl, NULL, 0) == 0); + } + int ret = mbedtls_ssl_read(ssl, buf + *read, buf_len - *read); if (ret > 0) { (*fragments)++;