Fix buffer overflow in GetRawInputDeviceInfo proxy

The proxy for GetRawInputDeviceInfo() incorrectly assumes that pcbSize
is always in wchars when unicode is used, which is only true if the
name of the device is being queried (`uiCommand == RIDI_DEVICENAME`).

Otherwise, it is the exact size of the buffer in bytes. Right now,
that means that the proxy will write double the number of bytes it's
supposed to for the other `uiCommand` values, potentially causing a
buffer overflow.

Also, when no `pData` value is given, `*pcbSize` is allowed to be
uninitialized. The proxy currently reads it unconditionally as a UINT,
which would be UB in that case. `memcpy` is able to safely copy
uninitialized values, so let's use that instead!

Sandboxie/core/dll/guimisc.c

@@ -1330,10 +1330,13 @@ _FX LONG Gui_GetRawInputDeviceInfo_impl(
     GUI_GET_RAW_INPUT_DEVICE_INFO_REQ* req;
     GUI_GET_RAW_INPUT_DEVICE_INFO_RPL* rpl;
 
-    // Note: pcbSize seems to be in tchars not in bytes!
     ULONG lenData = 0;
-    if (pData && pcbSize)
-        lenData = (*pcbSize) * (bUnicode ? sizeof(WCHAR) : 1);
+    if (pData && pcbSize) {
+        lenData = *pcbSize;
+        if (uiCommand == RIDI_DEVICENAME && bUnicode) {
+            lenData *= sizeof(WCHAR);
+        }
+    }
 
     ULONG reqSize = sizeof(GUI_GET_RAW_INPUT_DEVICE_INFO_REQ) + lenData + 10;
     req = Dll_Alloc(reqSize);
@@ -1344,12 +1347,19 @@ _FX LONG Gui_GetRawInputDeviceInfo_impl(
     req->hDevice = (ULONG64)hDevice;
     req->uiCommand = uiCommand;
     req->unicode = bUnicode;
+    req->hasData = !!pData;
+    req->hasSize = !!pcbSize;
+
+    if (pcbSize) {
+        // *pcbSize is allowed to be uninitialized if pData == nullptr.
+        // It would be UB to access it as a UINT, so it's important that
+        // we simply copy the bytes without interpretation.
+        memcpy(&req->cbSize, pcbSize, sizeof(*pcbSize));
+    }
+
     if (lenData) {
         memcpy(reqData, pData, lenData);
-        req->hasData = TRUE;
-    } else
-        req->hasData = FALSE;
-    req->cbSize = pcbSize ? *pcbSize : -1;
+    }
 
     rpl = Gui_CallProxy(req, reqSize, sizeof(*rpl));
 
@@ -1357,21 +1367,22 @@ _FX LONG Gui_GetRawInputDeviceInfo_impl(
 
     if (!rpl)
         return -1;
-    else {
-        ULONG error = rpl->error;
-        ULONG retval = rpl->retval;
 
-        if (pcbSize)
-            *pcbSize = rpl->cbSize;
-        if (lenData) {
-            LPVOID rplData = (BYTE*)rpl + sizeof(GUI_GET_RAW_INPUT_DEVICE_INFO_RPL);
-            memcpy(pData, rplData, lenData);
-        }
+    ULONG error = rpl->error;
+    ULONG retval = rpl->retval;
 
-        Dll_Free(rpl);
-        SetLastError(error);
-        return retval;
+    if (pcbSize) {
+        memcpy(pcbSize, &rpl->cbSize, sizeof(rpl->cbSize));
     }
+
+    if (lenData) {
+        LPVOID rplData = (BYTE*)rpl + sizeof(GUI_GET_RAW_INPUT_DEVICE_INFO_RPL);
+        memcpy(pData, rplData, lenData);
+    }
+
+    Dll_Free(rpl);
+    SetLastError(error);
+    return retval;
 }
 
 

Sandboxie/core/svc/GuiServer.cpp

@@ -3532,31 +3532,34 @@ ULONG GuiServer::GetRawInputDeviceInfoSlave(SlaveArgs *args)
         return STATUS_INFO_LENGTH_MISMATCH;
 
     LPVOID reqData = req->hasData ? (BYTE*)req + sizeof(GUI_GET_RAW_INPUT_DEVICE_INFO_REQ) : NULL;
-    PUINT pcbSize = NULL;
-    if (req->cbSize != -1)
-        pcbSize = &req->cbSize;
+    PUINT pcbSize = req->hasSize ? &req->cbSize : NULL;
+
+    ULONG lenData = 0;
+    if (reqData && pcbSize) {
+        lenData = *pcbSize;
+        if (req->uiCommand == RIDI_DEVICENAME && req->unicode) {
+            lenData *= sizeof(WCHAR);
+        }
+    }
 
     SetLastError(ERROR_SUCCESS);
     if (req->unicode) {
         rpl->retval = GetRawInputDeviceInfoW((HANDLE)req->hDevice, req->uiCommand, reqData, pcbSize);
-    }
-    else {
+    } else {
         rpl->retval = GetRawInputDeviceInfoA((HANDLE)req->hDevice, req->uiCommand, reqData, pcbSize);
     }
     rpl->error = GetLastError();
 
-    rpl->cbSize = req->cbSize;
-    if (pcbSize && req->hasData)
-    {
-        // Note: pcbSize seems to be in tchars not in bytes!
-        ULONG lenData = (*pcbSize) * (req->unicode ? sizeof(WCHAR) : 1);
+    if (pcbSize) {
+        // It's possible that (*pcbSize) could still be uninitialized.
+        // It would be UB to access it as a UINT.
+        memcpy(&rpl->cbSize, pcbSize, sizeof(*pcbSize));
+    }
 
-        rpl->hasData = TRUE;
+    if (lenData) {
         LPVOID rplData = (BYTE*)rpl + sizeof(GUI_GET_RAW_INPUT_DEVICE_INFO_RPL);
         memcpy(rplData, reqData, lenData);
     }
-    else
-        rpl->hasData = FALSE;
 
     args->rpl_len = args->req_len;
 

Sandboxie/core/svc/GuiWire.h

@@ -697,15 +697,14 @@ struct tagGUI_GET_RAW_INPUT_DEVICE_INFO_REQ
     UINT uiCommand;
     BOOLEAN unicode;
     BOOLEAN hasData;
+    BOOLEAN hasSize;
     UINT cbSize;
 };
 
 struct tagGUI_GET_RAW_INPUT_DEVICE_INFO_RPL
 {
-    ULONG status;
     ULONG error;
     ULONG retval;
-    BOOLEAN hasData;
     UINT cbSize;
 };