Changeset 97253 in vbox

Timestamp:

Oct 20, 2022 12:49:06 PM (2 years ago)

Author:

vboxsync

Message:

Shared Clipboard/common: Fixed a bug in ShClConvLatin1LFToUtf16CRLF() where a buffer overflow could happen, when handing-in strings which are bigger than the included string terminator (e.g. text which comes AFTER the string terminator). This caused overwriting areas bigger than the allocated pwszDst UTF-16 string, ultimately leading to a heap corruption.

File:

• trunk/src/VBox/GuestHost/SharedClipboard/clipboard-common.cpp (modified) (3 diffs)

trunk/src/VBox/GuestHost/SharedClipboard/clipboard-common.cpp

@@ -636 +636 @@
 }
 
+/**
+ * Converts a Latin-1 string with LF line endings into an UTF-16 string with CRLF endings.
+ *
+ * @returns VBox status code.
+ * @param   pcszSrc             Latin-1 string to convert.
+ * @param   cbSrc               Size (in bytes) of Latin-1 string to convert.
+ * @param   ppwszDst            Where to return the converted UTF-16 string on success.
+ * @param   pcwDst              Where to return the length (in UTF-16 characters) on success.
+ *
+ * @note    Only converts the source until the string terminator is found (or length limit is hit).
+ */
 int ShClConvLatin1LFToUtf16CRLF(const char *pcszSrc, size_t cbSrc,
                                 PRTUTF16 *ppwszDst, size_t *pcwDst)
@@ -644 +655 @@
     AssertPtrReturn(pcwDst,   VERR_INVALID_POINTER);
 
-    int rc = VINF_SUCCESS;
+    size_t chSrc = 0;
 
     PRTUTF16 pwszDst = NULL;
 
     /* Calculate the space needed. */
-    unsigned cwDst = 0;
-    for (unsigned i = 0; i < cbSrc && pcszSrc[i] != '\0'; ++i)
+    size_t cwDst = 0;
+    for (size_t i = 0; i < cbSrc && pcszSrc[i] != '\0'; ++i)
     {
         if (pcszSrc[i] == VBOX_SHCL_LINEFEED)
@@ -656 +667 @@
         else
             ++cwDst;
+        chSrc++;
     }
 
     pwszDst = (PRTUTF16)RTMemAlloc((cwDst + 1 /* Leave space for the terminator */) * sizeof(RTUTF16));
-    if (!pwszDst)
-        rc = VERR_NO_MEMORY;
+    AssertPtrReturn(pwszDst, VERR_NO_MEMORY);
 
     /* Do the conversion, bearing in mind that Latin-1 expands "naturally" to UTF-16. */
-    if (RT_SUCCESS(rc))
-    {
-        for (unsigned i = 0, j = 0; i < cbSrc; ++i, ++j)
-        {
-            if (pcszSrc[i] != VBOX_SHCL_LINEFEED)
-                pwszDst[j] = pcszSrc[i];
-            else
-            {
-                pwszDst[j]     = VBOX_SHCL_CARRIAGERETURN;
-                pwszDst[j + 1] = VBOX_SHCL_LINEFEED;
-                ++j;
-            }
-        }
-
-        pwszDst[cwDst] = '\0';  /* Make sure we are zero-terminated. */
-    }
-
-    if (RT_SUCCESS(rc))
-    {
-        *ppwszDst = pwszDst;
-        *pcwDst   = cwDst;
-    }
-    else
-        RTMemFree(pwszDst);
-
-    return rc;
+    for (size_t i = 0, j = 0; i < chSrc; ++i, ++j)
+    {
+        AssertMsg(j <= cwDst, ("cbSrc=%zu, j=%u vs. cwDst=%u\n", cbSrc, j, cwDst));
+        if (pcszSrc[i] != VBOX_SHCL_LINEFEED)
+            pwszDst[j] = pcszSrc[i];
+        else
+        {
+            pwszDst[j]     = VBOX_SHCL_CARRIAGERETURN;
+            pwszDst[j + 1] = VBOX_SHCL_LINEFEED;
+            ++j;
+        }
+    }
+
+    pwszDst[cwDst] = '\0';  /* Make sure we are zero-terminated. */
+
+    *ppwszDst = pwszDst;
+    *pcwDst   = cwDst;
+
+    return VINF_SUCCESS;
 }