Changeset 100521 in vbox

Timestamp:

Jul 11, 2023 4:47:42 PM (17 months ago)

Author:

vboxsync

Message:

Main: separate function for parsing the entries in remote USB device list. bugref:10478

Location:

trunk/src/VBox/Main

Files:

• include/RemoteUSBDeviceImpl.h (modified) (1 diff)
• src-client/ConsoleImpl.cpp (modified) (3 diffs)
• src-client/RemoteUSBBackend.cpp (modified) (1 diff)
• src-client/RemoteUSBDeviceImpl.cpp (modified) (1 diff)

trunk/src/VBox/Main/include/RemoteUSBDeviceImpl.h

@@ -51 +51 @@
 
     // public initializer/uninitializer for internal purposes only
-    HRESULT init(uint32_t u32ClientId, VRDEUSBDEVICEDESC *pDevDesc, bool fDescExt);
+    HRESULT init(uint32_t u32ClientId, VRDEUSBDEVICEDESC const *pDevDesc, bool fDescExt);
     void uninit();
 

trunk/src/VBox/Main/src-client/ConsoleImpl.cpp

@@ -10765 +10765 @@
 }
 
+/* Make sure that the string is null-terminated and its size is less than cchMax bytes.
+ * Replace invalid UTF8 bytes with '?'.
+ */
+static int validateUtf8String(char *psz, size_t cchMax)
+{
+    for (;;)
+    {
+        RTUNICP Cp;
+        int vrc = RTStrGetCpNEx((const char **)&psz, &cchMax, &Cp);
+        if (RT_SUCCESS(vrc))
+        {
+            if (!Cp)
+                break;
+        }
+        else
+        {
+            if (!cchMax)
+                return VERR_END_OF_STRING;
+
+            psz[-1] = '?';
+        }
+    }
+    return VINF_SUCCESS;
+}
+
+static int validateRemoteUSBDeviceDesc(VRDEUSBDEVICEDESC const *e, uint32_t cbRemaining, bool fDescExt)
+{
+    uint32_t const cbDesc = fDescExt ? sizeof(VRDEUSBDEVICEDESCEXT) : sizeof(VRDEUSBDEVICEDESC);
+    if (cbDesc > cbRemaining)
+        return VERR_INVALID_PARAMETER;
+
+    if (   e->oNext         >  cbRemaining /* It is OK for oNext to point to the end of buffer. */
+        || e->oManufacturer >= cbRemaining
+        || e->oProduct      >= cbRemaining
+        || e->oSerialNumber >= cbRemaining)
+        return VERR_INVALID_PARAMETER;
+
+    int vrc;
+    if (e->oManufacturer)
+    {
+        vrc = validateUtf8String((char *)e + e->oManufacturer, cbRemaining - e->oManufacturer);
+        if (RT_FAILURE(vrc))
+            return VERR_INVALID_PARAMETER;
+    }
+    if (e->oProduct)
+    {
+        vrc = validateUtf8String((char *)e + e->oProduct, cbRemaining - e->oProduct);
+        if (RT_FAILURE(vrc))
+            return VERR_INVALID_PARAMETER;
+    }
+    if (e->oSerialNumber)
+    {
+        vrc = validateUtf8String((char *)e + e->oSerialNumber, cbRemaining - e->oSerialNumber);
+        if (RT_FAILURE(vrc))
+            return VERR_INVALID_PARAMETER;
+    }
+
+    return VINF_SUCCESS;
+}
+
 /**
  * @note Locks this object for writing.
@@ -10799 +10859 @@
      * Process the pDevList and add devices those are not already in the mRemoteUSBDevices list.
      */
-    /** @todo (sunlover) REMOTE_USB Strict validation of the pDevList. */
     VRDEUSBDEVICEDESC *e = pDevList;
-
-    /* The cbDevList condition must be checked first, because the function can
+    uint32_t cbRemaining = cbDevList;
+
+    /* The cbRemaining condition must be checked first, because the function can
      * receive pDevList = NULL and cbDevList = 0 on client disconnect.
      */
-    while (cbDevList >= 2 && e->oNext)
-    {
-        /* Sanitize incoming strings in case they aren't valid UTF-8. */
-        if (e->oManufacturer)
-            RTStrPurgeEncoding((char *)e + e->oManufacturer);
-        if (e->oProduct)
-            RTStrPurgeEncoding((char *)e + e->oProduct);
-        if (e->oSerialNumber)
-            RTStrPurgeEncoding((char *)e + e->oSerialNumber);
+    while (cbRemaining >= sizeof(e->oNext) && e->oNext)
+    {
+        int const vrc = validateRemoteUSBDeviceDesc(e, cbRemaining, fDescExt);
+        if (RT_FAILURE(vrc))
+            break; /* Consider the rest of the list invalid too. */
 
         LogFlowThisFunc(("vendor %04x, product %04x, name = %s\n",
@@ -10871 +10927 @@
         }
 
-        if (cbDevList < e->oNext)
-        {
-            Log1WarningThisFunc(("cbDevList %d > oNext %d\n", cbDevList, e->oNext));
-            break;
-        }
-
-        cbDevList -= e->oNext;
+        AssertBreak(cbRemaining >= e->oNext); /* validateRemoteUSBDeviceDesc ensures this. */
+        cbRemaining -= e->oNext;
 
         e = (VRDEUSBDEVICEDESC *)((uint8_t *)e + e->oNext);

trunk/src/VBox/Main/src-client/RemoteUSBBackend.cpp

@@ -1107 +1107 @@
         RTMemFree(mpvDeviceList);
         mpvDeviceList = NULL;
-
-        mcbDeviceList = cbList;
+        mcbDeviceList = 0;
 
         if (cbList > 0)
         {
             mpvDeviceList = RTMemAlloc(cbList);
+            AssertPtrReturn(mpvDeviceList, VERR_NO_MEMORY);
+
             memcpy(mpvDeviceList, pvList, cbList);
+            mcbDeviceList = cbList;
         }
 

trunk/src/VBox/Main/src-client/RemoteUSBDeviceImpl.cpp

@@ -65 +65 @@
  * Initializes the remote USB device object.
  */
-HRESULT RemoteUSBDevice::init(uint32_t u32ClientId, VRDEUSBDEVICEDESC *pDevDesc, bool fDescExt)
+HRESULT RemoteUSBDevice::init(uint32_t u32ClientId, VRDEUSBDEVICEDESC const *pDevDesc, bool fDescExt)
 {
     LogFlowThisFunc(("u32ClientId=%d,pDevDesc=%p\n", u32ClientId, pDevDesc));