[PATCH v2 1/1] shell32: Avoid stack corruption caused by long URLs

[Stefan Rentsch]

From: Stefan Rentsch

---
 dlls/shell32/shlexec.c       | 29 ++++++++++++++++++++++++-----
 dlls/shell32/tests/shlexec.c | 29 +++++++++++++++++++++++++----
 2 files changed, 49 insertions(+), 9 deletions(-)

diff --git a/dlls/shell32/shlexec.c b/dlls/shell32/shlexec.c
index da2b5fdc2b0..e69f238445d 100644
--- a/dlls/shell32/shlexec.c
+++ b/dlls/shell32/shlexec.c
@@ -422,13 +422,22 @@ static void *SHELL_BuildEnvW( const WCHAR *path )
 static BOOL SHELL_TryAppPathW( LPCWSTR szName, LPWSTR lpResult, WCHAR **env)
 {
     HKEY hkApp = 0;
-    WCHAR buffer[1024];
+    WCHAR *buffer;
+    LONG bufferLen;
+    LPCWSTR keyPath = L"Software\Microsoft\Windows\CurrentVersion\App Paths\";
     LONG len;
     LONG res;
     BOOL found = FALSE;

if (env) *env = NULL;
-    lstrcpyW(buffer, L"Software\Microsoft\Windows\CurrentVersion\App Paths\");
+
+    bufferLen = lstrlenW(keyPath) + lstrlenW(szName) + 1;
+    /* At least (2 * MAX_PATH) to hold the returned path from the registry */
+    if (bufferLen < 2 * MAX_PATH)
+        bufferLen = 2 * MAX_PATH;
+
+    buffer = heap_alloc(bufferLen * sizeof(WCHAR));
+    lstrcpyW(buffer, keyPath);
     lstrcatW(buffer, szName);
     res = RegOpenKeyExW(HKEY_LOCAL_MACHINE, buffer, 0, KEY_READ, &hkApp);
     if (res) goto end;
@@ -440,12 +449,13 @@ static BOOL SHELL_TryAppPathW( LPCWSTR szName, LPWSTR lpResult, WCHAR **env)

if (env)
     {
-        DWORD count = sizeof(buffer);
+        DWORD count = bufferLen * sizeof(WCHAR);
         if (!RegQueryValueExW(hkApp, L"Path", NULL, NULL, (LPBYTE)buffer, &count) && buffer[0])
             *env = SHELL_BuildEnvW( buffer );
     }

end:
+    heap_free(buffer);
     if (hkApp) RegCloseKey(hkApp);
     return found;
 }
@@ -1763,8 +1773,17 @@ static BOOL SHELL_execute( LPSHELLEXECUTEINFOW sei, SHELL_ExecuteW32 execfunc )
     /* Else, try to execute the filename */
     TRACE("execute:%s,%s,%s\n", debugstr_w(wszApplicationName), debugstr_w(wszParameters), debugstr_w(wszDir));
     lpFile = sei_tmp.lpFile;
-    wcmd = wcmdBuffer;
-    lstrcpyW(wcmd, wszApplicationName);
+    {
+        size_t bufferLength = ARRAY_SIZE(wcmdBuffer) - 1;
+        INT sourceLength = lstrlenW(wszApplicationName);
+
+        if (sourceLength > bufferLength)
+            ERR("stack too small for application: %s\n", debugstr_w(wszApplicationName));
+
+        wcmd = wcmdBuffer;
+        lstrcpynW(wcmd, wszApplicationName, bufferLength);
+    }
+
     if (sei_tmp.lpDirectory)
     {
         LPCWSTR searchPath[] = {
diff --git a/dlls/shell32/tests/shlexec.c b/dlls/shell32/tests/shlexec.c
index b84a96a2434..030d06f71be 100644
--- a/dlls/shell32/tests/shlexec.c
+++ b/dlls/shell32/tests/shlexec.c
@@ -119,7 +119,7 @@ static void WINAPIV __WINE_PRINTF_ATTR(2,3) childPrintf(HANDLE h, const char* fm
     DWORD       w;

va_start(valist, fmt);
-    vsprintf(buffer, fmt, valist);
+    vsnprintf(buffer, ARRAY_SIZE(buffer), fmt, valist);
     va_end(valist);
     WriteFile(h, buffer, strlen(buffer), &w, NULL);
 }
@@ -348,16 +348,21 @@ static void dump_child_(const char* file, int line)
  *
  ***/

-static char shell_call[2048];
+static char shell_call[2048 * 2];
 static void WINAPIV __WINE_PRINTF_ATTR(2,3) _okShell(int condition, const char *msg, ...)
 {
     va_list valist;
     char buffer[2048];
+    size_t pos = 0;

-    strcpy(buffer, shell_call);
+    lstrcpynA(buffer, shell_call, MAX_PATH);
+    if (strlen(buffer) == MAX_PATH)
+        strcat(buffer, "(...)");
     strcat(buffer, " ");
+    pos = strlen(buffer);
+
     va_start(valist, msg);
-    vsprintf(buffer+strlen(buffer), msg, valist);
+    vsnprintf(buffer + pos, ARRAY_SIZE(buffer) - pos - 1, msg, valist);
     va_end(valist);
     winetest_ok(condition, "%s", buffer);
 }
@@ -1919,6 +1924,22 @@ static void test_urls(void)
     okChildString("argvA3", "URL");
     okChildString("argvA4", "shlproto://foo/bar");

+    {
+        /* Test against compatibility of long URI */
+        char long_uri[2100];
+        memset(long_uri, 0, sizeof(long_uri));
+        strcpy(long_uri, "shlproto://foo/bar");
+        memset(long_uri + strlen(long_uri), 'r', sizeof(long_uri) - strlen(long_uri) - 5);
+        strcat(long_uri, ".exe");
+        assert(strlen(long_uri) < sizeof(long_uri));
+
+        rc = shell_execute(NULL, long_uri, NULL, NULL);
+        ok(rc > 32, "%s failed: rc=%Iu\n", shell_call, rc);
+        okChildInt("argcA", 5);
+        okChildString("argvA3", "URL");
+        todo_wine okChildString("argvA4", long_uri);
+    }
+
     /* Check default verb detection */
     rc = shell_execute(NULL, "shlpaverb://foo/bar", NULL, NULL);
     todo_wine ok(rc > 32 || /* XP+IE7 - Win10 */