From 5be4dff8b481f80d8006a69eb3312af458a1acde Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?=
 <marmarek@invisiblethingslab.com>
Date: Sat, 27 Jan 2024 17:14:19 +0100
Subject: [PATCH] qrexec: try service name without argument for long arguments

Do not fail immediately if service name with the argument is too long
for the path name, but try the name without the argument anyway.

While at it, fix missing error handling for StringCchPrintf.

Part of QubesOS/qubes-issues#4909
---
 src/qrexec-agent/qrexec-agent.c | 66 ++++++++++++++++++++++++---------
 1 file changed, 48 insertions(+), 18 deletions(-)

diff --git a/src/qrexec-agent/qrexec-agent.c b/src/qrexec-agent/qrexec-agent.c
index 137ffdb..dcecd6d 100644
--- a/src/qrexec-agent/qrexec-agent.c
+++ b/src/qrexec-agent/qrexec-agent.c
@@ -492,24 +492,34 @@ static DWORD InterceptRPCRequest(IN OUT WCHAR *commandLine, OUT WCHAR **serviceC
         // Leave trailing backslash
         separator++;
         *separator = L'\0';
-        if (wcslen(serviceFilePath) + wcslen(L"qubes-rpc\\") + wcslen(serviceName) > MAX_PATH)
+        if (wcslen(serviceFilePath) + wcslen(L"qubes-rpc\\_") > MAX_PATH)
         {
             free(*sourceDomainName);
             LogError("RPC service config file path too long");
             return ERROR_PATH_NOT_FOUND;
         }
-
         PathAppendW(serviceFilePath, L"qubes-rpc");
-        PathAppendW(serviceFilePath, serviceName);
 
-        serviceConfigFile = CreateFile(
-            serviceFilePath,    // file to open
-            GENERIC_READ,          // open for reading
-            FILE_SHARE_READ,       // share for reading
-            NULL,                  // default security
-            OPEN_EXISTING,         // existing file only
-            FILE_ATTRIBUTE_NORMAL, // normal file
-            NULL);                 // no attr. template
+        if (wcslen(serviceFilePath) + wcslen(serviceName) > MAX_PATH)
+        {
+            LogError("RPC service config file path too long, trying without argument");
+            // append dummy service for uniform handling below
+            PathAppendW(serviceFilePath, L"_");
+            serviceConfigFile = INVALID_HANDLE_VALUE;
+        }
+        else
+        {
+            PathAppendW(serviceFilePath, serviceName);
+
+            serviceConfigFile = CreateFile(
+                serviceFilePath,    // file to open
+                GENERIC_READ,          // open for reading
+                FILE_SHARE_READ,       // share for reading
+                NULL,                  // default security
+                OPEN_EXISTING,         // existing file only
+                FILE_ATTRIBUTE_NORMAL, // normal file
+                NULL);                 // no attr. template
+        }
 
         if (serviceConfigFile == INVALID_HANDLE_VALUE)
         {
@@ -531,6 +541,13 @@ static DWORD InterceptRPCRequest(IN OUT WCHAR *commandLine, OUT WCHAR **serviceC
                 }
                 newsep++;
                 *newsep = L'\0';
+                if (wcslen(serviceFilePath) + wcslen(serviceName) > MAX_PATH)
+                {
+                    free(*sourceDomainName);
+                    LogError("RPC service config file path too long");
+                    return ERROR_PATH_NOT_FOUND;
+                }
+
                 PathAppendW(serviceFilePath, serviceName);
 
                 serviceConfigFile = CreateFile(
@@ -683,7 +700,15 @@ BOOL VchanSendHello(
  */
 static DWORD StartChild(int domain, int port, PWSTR userName, PWSTR commandLine, BOOL isServer, BOOL piped, BOOL interactive)
 {
-    PWSTR command = malloc(MAX_PATH_LONG * sizeof(WCHAR));
+    size_t const commandlen = wcslen(L"qrexec-wrapper.exe ") +
+                              10 + // domain (int)
+                              10 + // port (int)
+                              wcslen(userName ? : L"(null)") +
+                              1 + // flags (see below)
+                              wcslen(commandLine ? : L"(null)") +
+                              4 + // separators
+                              1; // final NUL
+    PWSTR command = malloc(commandlen * sizeof(WCHAR));
     int flags = 0;
     HANDLE wrapper;
     DWORD status;
@@ -705,12 +730,17 @@ static DWORD StartChild(int domain, int port, PWSTR userName, PWSTR commandLine,
     if (piped)       flags |= 0x02;
     if (interactive) flags |= 0x04;
 
-    StringCchPrintf(command, MAX_PATH_LONG, L"qrexec-wrapper.exe %d%c%d%c%s%c%d%c%s",
-                    domain, QUBES_ARGUMENT_SEPARATOR,
-                    port, QUBES_ARGUMENT_SEPARATOR,
-                    userName, QUBES_ARGUMENT_SEPARATOR,
-                    flags, QUBES_ARGUMENT_SEPARATOR,
-                    commandLine);
+    if (FAILED(StringCchPrintf(command, commandlen, L"qrexec-wrapper.exe %d%c%d%c%s%c%d%c%s",
+                               domain, QUBES_ARGUMENT_SEPARATOR,
+                               port, QUBES_ARGUMENT_SEPARATOR,
+                               userName, QUBES_ARGUMENT_SEPARATOR,
+                               flags, QUBES_ARGUMENT_SEPARATOR,
+                               commandLine)))
+    {
+        free(command);
+        LogError("Failed to prepare qrexec-wrapper command");
+        return ERROR_NOT_ENOUGH_MEMORY;
+    }
 
     // wrapper will run as current user (SYSTEM, we're a service)
     status = CreateNormalProcessAsCurrentUser(command, &wrapper);