commit 756b0285119f872eea025e9d6888c9f88307ff9d
author Dave Rodgman <dave.rodgman@arm.com> Mon Mar 13 10:46:29 2023 +0000
committer GitHub <noreply@github.com> Mon Mar 13 10:46:29 2023 +0000
tree ee7a9998acf6196c2eda1b3b38bfcd15a62a2f8b
parent 3ef7a6af1217015bd0fe1ec6d1259e58715d6368
parent a4e8fb0041191e75970cb2e2b1ea70b915b87afb

Merge pull request #7171 from daverodgman/pr5527

Fix undefined behavior in ssl_read if buf parameter is NULL

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 0000000..1f2c563
--- /dev/null
+++ b/ChangeLog.d/mbedtls_ssl_read_undefined_behavior.txt
@@ -0,0 +1,3 @@
+Bugfix
+   * Fix undefined behavior in mbedtls_ssl_read() and mbedtls_ssl_write() if
+     len argument is 0 and buffer is NULL.

library/ssl_msg.c

diff --git a/library/ssl_msg.c b/library/ssl_msg.c
index 1cad588..18c19f9 100644
--- a/library/ssl_msg.c
+++ b/library/ssl_msg.c
@@ -5599,8 +5599,10 @@
     n = (len < ssl->in_msglen)
         ? len : ssl->in_msglen;
 
-    memcpy(buf, ssl->in_offt, n);
-    ssl->in_msglen -= n;
+    if (len != 0) {
+        memcpy(buf, ssl->in_offt, n);
+        ssl->in_msglen -= n;
+    }
 
     /* Zeroising the plaintext buffer to erase unused application data
        from the memory. */
@@ -5676,7 +5678,9 @@
          */
         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);

tests/suites/test_suite_ssl.function

diff --git a/tests/suites/test_suite_ssl.function b/tests/suites/test_suite_ssl.function
index f486021..9dfb7ed 100644
--- a/tests/suites/test_suite_ssl.function
+++ b/tests/suites/test_suite_ssl.function
@@ -1165,6 +1165,12 @@
                                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;
@@ -1203,6 +1209,12 @@
                               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)++;