Merge pull request #7172 from daverodgman/fix_UB_in_ssl_read-2.28

Backport 2.28: Fix undefined behavior in ssl_read if buf parameter is NULL
Mbed-TLS · Mar 13, 2023 · 39987eb · 39987eb
2 parents ed36bbe + cd09d68
commit 39987eb
Show file tree

Hide file tree

Showing 3 changed files with 22 additions and 3 deletions.
diff --git a/ChangeLog.d/mbedtls_ssl_read_undefined_behavior.txt 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.
diff --git a/library/ssl_msg.c b/library/ssl_msg.c
@@ -5429,8 +5429,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 (len != 0) {
+        memcpy(buf, ssl->in_offt, n);
+        ssl->in_msglen -= n;
+    }
 
     /* Zeroising the plaintext buffer to erase unused application data
        from the memory. */
@@ -5506,7 +5508,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);

diff --git a/tests/suites/test_suite_ssl.function b/tests/suites/test_suite_ssl.function
@@ -1052,6 +1052,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;
@@ -1090,6 +1096,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)++;