[PATCH v2 0/1] MR7872: shell32: Avoid writing past end of xlpFile or lpResult in SHELL_FindExecutable().

[Paul Gofman (＠gofman)]

--
  v2: shell32: Avoid writing past end of xlpFile or lpResult in SHELL_FindExecutable().
 
https://gitlab.winehq.org/wine/wine/-/merge_requests/7872

[PATCH v2 1/1] shell32: Avoid writing past end of xlpFile or lpResult in SHELL_FindExecutable().

[Paul Gofman]

From: Paul Gofman <pgofman(a)codeweavers.com>

---
 dlls/shell32/shlexec.c | 25 ++++++++++++++++++-------
 1 file changed, 18 insertions(+), 7 deletions(-)

diff --git a/dlls/shell32/shlexec.c b/dlls/shell32/shlexec.c
index 956b844b572..3db66aa7bb9 100644
--- a/dlls/shell32/shlexec.c
+++ b/dlls/shell32/shlexec.c
@@ -643,7 +643,9 @@ static UINT SHELL_FindExecutable(LPCWSTR lpPath, LPCWSTR lpFile, LPCWSTR lpVerb,
         }
         if (!found)
         {
-            lstrcpyW(xlpFile, lpFile);
+            if (wcsncpy_s(xlpFile, ARRAY_SIZE(xlpFile), lpFile, _TRUNCATE))
+                return ERROR_FILENAME_EXCED_RANGE;
+
             if (PathFileExistsDefExtW(xlpFile, 0xbf) || PathFileExistsW(xlpFile))
             {
                 GetFullPathNameW(xlpFile, ARRAY_SIZE(xlpFile), xlpFile, NULL);
@@ -653,7 +655,8 @@ static UINT SHELL_FindExecutable(LPCWSTR lpPath, LPCWSTR lpFile, LPCWSTR lpVerb,
         if (found)
         {
             lpFile = xlpFile;
-            lstrcpyW(lpResult, xlpFile);
+            if (wcsncpy_s(lpResult, resultLen, xlpFile, _TRUNCATE))
+                return ERROR_FILENAME_EXCED_RANGE;
         }
         else
             xlpFile[0] = '\0';
@@ -667,13 +670,17 @@ static UINT SHELL_FindExecutable(LPCWSTR lpPath, LPCWSTR lpFile, LPCWSTR lpVerb,
         }
         else
             search_paths[0] = curdir;
-        lstrcpyW(xlpFile, lpFile);
+
+        if (wcsncpy_s(xlpFile, ARRAY_SIZE(xlpFile), lpFile, _TRUNCATE))
+            return ERROR_FILENAME_EXCED_RANGE;
+
         if (PathResolveW(xlpFile, search_paths, PRF_TRYPROGRAMEXTENSIONS | PRF_VERIFYEXISTS) ||
             PathFindOnPathW(xlpFile, search_paths))
         {
             TRACE("PathResolveAW returned non-zero\n");
             lpFile = xlpFile;
-            lstrcpyW(lpResult, xlpFile);
+            if (wcsncpy_s(lpResult, resultLen, xlpFile, _TRUNCATE))
+                return ERROR_FILENAME_EXCED_RANGE;
             /* The file was found in lpPath or one of the directories in the system-wide search path */
         }
         else
@@ -799,16 +806,20 @@ static UINT SHELL_FindExecutable(LPCWSTR lpPath, LPCWSTR lpFile, LPCWSTR lpVerb,
         {
             if (*command)
             {
-                lstrcpyW(lpResult, command);
+                if (wcsncpy_s(lpResult, resultLen, command, _TRUNCATE))
+                    return ERROR_FILENAME_EXCED_RANGE;
                 tok = wcschr(lpResult, '^'); /* should be ^.extension? */
                 if (tok != NULL)
                 {
                     tok[0] = '\0';
-                    lstrcatW(lpResult, xlpFile); /* what if no dir in xlpFile? */
+                    /* what if no dir in xlpFile? */
+                    if (wcscat_s(lpResult, resultLen, xlpFile))
+                        return ERROR_FILENAME_EXCED_RANGE;
                     tok = wcschr(command, '^'); /* see above */
                     if ((tok != NULL) && (lstrlenW(tok)>5))
                     {
-                        lstrcatW(lpResult, &tok[5]);
+                        if (wcscat_s(lpResult, resultLen, &tok[5]))
+                            return ERROR_FILENAME_EXCED_RANGE;
                     }
                 }
                 retval = 33; /* FIXME - see above */
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/7872

[Paul Gofman (＠gofman)]

v2:
 - Return an error instead of silently truncating.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/7872#note_101881

[Alexandre Julliard (＠julliard)]

Error codes need to be <= 32 here.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/7872#note_102771