[PATCH v7 0/1] MR11387: start: Avoid buffer overflows during path lookup.
start.exe searches for its first argument using fixed-size path buffers. Several normalized paths, path components, executable names, and PATHEXT entries were copied without consistent bounds checks, allowing a long protocol URL to cause an access violation before ShellExecuteExW. Validate each destination buffer operation and leave arguments that cannot be represented by the path search buffers unchanged for ShellExecuteExW. The later fix update adds a cmd.exe conformance test. It registers a temporary protocol and verifies that cmd.exe /c start passes a URL longer than MAX_PATH to its handler unchanged. Related merge request: !11386 adds per-user protocol lookup and desktop export for handlers registered under HKCU\\Software\\Classes. End-to-end Notion Calendar testing was performed with this MR and !11386 applied together. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=57229 Signed-off-by: Likun Li lilikun124@gmail.com -- v7: start: Avoid buffer overflows during path lookup. https://gitlab.winehq.org/wine/wine/-/merge_requests/11387
From: lily11111222 <2564065295@qq.com> search_path() copies normalized paths, path components, executable names, and PATHEXT entries into fixed-size buffers without consistently checking their lengths. A long URL can therefore overflow the buffers and cause an access violation before it reaches ShellExecuteExW. Validate each string before copying or concatenating it, and handle paths without a backslash. If an argument cannot be represented by the path search buffers, leave it unchanged for ShellExecuteExW to handle. Add a cmd.exe test which registers a temporary protocol and verifies that start can launch a URL longer than MAX_PATH. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=57229 Signed-off-by: lily11111222 <2564065295@qq.com> --- programs/cmd/tests/test_builtins.bat | 21 +++++++++++++++++ programs/cmd/tests/test_builtins.bat.exp | 1 + programs/cmd/tests/test_builtins.cmd | 21 +++++++++++++++++ programs/cmd/tests/test_builtins.cmd.exp | 1 + programs/start/start.c | 30 +++++++++++++++++++----- 5 files changed, 68 insertions(+), 6 deletions(-) diff --git a/programs/cmd/tests/test_builtins.bat b/programs/cmd/tests/test_builtins.bat index 7711aa917e3..995c1fa4042 100644 --- a/programs/cmd/tests/test_builtins.bat +++ b/programs/cmd/tests/test_builtins.bat @@ -97,6 +97,27 @@ set "FOO_PATH=%cd%" > NUL cd .. call :setError 666 & (start /B /WAIT /d "%FOO_PATH%" cmd /s /c "if /I \"%%cd%%\"==\"%FOO_PATH%\" (exit 0) else (exit 1)" >nul &&echo !errorlevel!) rd /q /s foo + +rem Ensure START handles a protocol URL longer than MAX_PATH. +set "WINE_START_PROTOCOL=wineteststart%random%%random%" +set "WINE_START_URL=%WINE_START_PROTOCOL%://callback/" +for /l %%i in (1,1,512) do set "WINE_START_URL=!WINE_START_URL!a" +set "WINE_START_RESULT=%cd%\start_result.txt" +del /q "%WINE_START_RESULT%" >nul 2>&1 +reg add "HKCU\Software\Classes\%WINE_START_PROTOCOL%" /ve /d "URL:%WINE_START_PROTOCOL%" /f >nul +reg add "HKCU\Software\Classes\%WINE_START_PROTOCOL%" /v "URL Protocol" /d "" /f >nul +reg add "HKCU\Software\Classes\%WINE_START_PROTOCOL%\shell\open\command" /ve ^ + /d "%comspec% /d /c type nul > %WINE_START_RESULT%" /f >nul +reg add "HKCR\%WINE_START_PROTOCOL%" /ve /d "URL:%WINE_START_PROTOCOL%" /f >nul 2>&1 +reg add "HKCR\%WINE_START_PROTOCOL%" /v "URL Protocol" /d "" /f >nul 2>&1 +reg add "HKCR\%WINE_START_PROTOCOL%\shell\open\command" /ve ^ + /d "%comspec% /d /c type nul > %WINE_START_RESULT%" /f >nul 2>&1 +start "" /b /wait !WINE_START_URL! >nul 2>&1 +if exist "%WINE_START_RESULT%" (echo long protocol URL handled) else echo protocol handler was not invoked +reg delete "HKCR\%WINE_START_PROTOCOL%" /f >nul 2>&1 +reg delete "HKCU\Software\Classes\%WINE_START_PROTOCOL%" /f >nul 2>&1 +del /q "%WINE_START_RESULT%" >nul 2>&1 + echo --- success/failure for TYPE command mkdir foo & cd foo mkdir bar diff --git a/programs/cmd/tests/test_builtins.bat.exp b/programs/cmd/tests/test_builtins.bat.exp index 77ca71c8778..002bc6b3ed7 100644 --- a/programs/cmd/tests/test_builtins.bat.exp +++ b/programs/cmd/tests/test_builtins.bat.exp @@ -63,6 +63,7 @@ foo@space@ SUCCESS 1024 @todo_wine@SUCCESS 666 0 +long protocol URL handled --- success/failure for TYPE command FAILURE 1 SUCCESS 0 diff --git a/programs/cmd/tests/test_builtins.cmd b/programs/cmd/tests/test_builtins.cmd index 34d1f076485..344368a0896 100644 --- a/programs/cmd/tests/test_builtins.cmd +++ b/programs/cmd/tests/test_builtins.cmd @@ -680,6 +680,27 @@ set "FOO_PATH=%cd%" > NUL cd .. call :setError 666 & (start /B /WAIT /d "%FOO_PATH%" cmd /s /c "if /I \"%%cd%%\"==\"%FOO_PATH%\" (exit 0) else (exit 1)" >nul &&echo !errorlevel!) rd /q /s foo + +rem Ensure START handles a protocol URL longer than MAX_PATH. +set "WINE_START_PROTOCOL=wineteststart%random%%random%" +set "WINE_START_URL=%WINE_START_PROTOCOL%://callback/" +for /l %%i in (1,1,512) do set "WINE_START_URL=!WINE_START_URL!a" +set "WINE_START_RESULT=%cd%\start_result.txt" +del /q "%WINE_START_RESULT%" >nul 2>&1 +reg add "HKCU\Software\Classes\%WINE_START_PROTOCOL%" /ve /d "URL:%WINE_START_PROTOCOL%" /f >nul +reg add "HKCU\Software\Classes\%WINE_START_PROTOCOL%" /v "URL Protocol" /d "" /f >nul +reg add "HKCU\Software\Classes\%WINE_START_PROTOCOL%\shell\open\command" /ve ^ + /d "%comspec% /d /c type nul > %WINE_START_RESULT%" /f >nul +reg add "HKCR\%WINE_START_PROTOCOL%" /ve /d "URL:%WINE_START_PROTOCOL%" /f >nul 2>&1 +reg add "HKCR\%WINE_START_PROTOCOL%" /v "URL Protocol" /d "" /f >nul 2>&1 +reg add "HKCR\%WINE_START_PROTOCOL%\shell\open\command" /ve ^ + /d "%comspec% /d /c type nul > %WINE_START_RESULT%" /f >nul 2>&1 +start "" /b /wait !WINE_START_URL! >nul 2>&1 +if exist "%WINE_START_RESULT%" (echo long protocol URL handled) else echo protocol handler was not invoked +reg delete "HKCR\%WINE_START_PROTOCOL%" /f >nul 2>&1 +reg delete "HKCU\Software\Classes\%WINE_START_PROTOCOL%" /f >nul 2>&1 +del /q "%WINE_START_RESULT%" >nul 2>&1 + echo --- success/failure for TYPE command mkdir foo & cd foo mkdir bar diff --git a/programs/cmd/tests/test_builtins.cmd.exp b/programs/cmd/tests/test_builtins.cmd.exp index dc6420e1432..274495a4cb7 100644 --- a/programs/cmd/tests/test_builtins.cmd.exp +++ b/programs/cmd/tests/test_builtins.cmd.exp @@ -582,6 +582,7 @@ foo@space@ SUCCESS 1024 @todo_wine@SUCCESS 666 0 +long protocol URL handled --- success/failure for TYPE command FAILURE 1 SUCCESS 0 diff --git a/programs/start/start.c b/programs/start/start.c index 38aafe870a9..2940f44640d 100644 --- a/programs/start/start.c +++ b/programs/start/start.c @@ -290,19 +290,21 @@ static BOOL search_path(const WCHAR *firstParam, WCHAR **full_path) lstrcpyW (pathtosearch, L"."); } if (wcschr(firstParam, '.') != NULL) extensionsupplied = TRUE; - if (lstrlenW(firstParam) >= MAX_PATH) { + if (lstrlenW(firstParam) >= ARRAY_SIZE(stemofsearch)) return FALSE; - } - lstrcpyW(stemofsearch, firstParam); } else { /* Convert eg. ..\fred to include a directory by removing file part */ - GetFullPathNameW(firstParam, ARRAY_SIZE(pathtosearch), pathtosearch, NULL); + len = GetFullPathNameW(firstParam, ARRAY_SIZE(pathtosearch), pathtosearch, NULL); + if (!len || len >= ARRAY_SIZE(pathtosearch)) + return FALSE; lastSlash = wcsrchr(pathtosearch, '\\'); if (lastSlash && wcschr(lastSlash, '.') != NULL) extensionsupplied = TRUE; - lstrcpyW(stemofsearch, lastSlash+1); + if (lstrlenW(lastSlash ? lastSlash + 1 : firstParam) >= ARRAY_SIZE(stemofsearch)) + return FALSE; + lstrcpyW(stemofsearch, lastSlash ? lastSlash + 1 : firstParam); /* Reduce pathtosearch to a path with trailing '\' to support c:\a.bat and c:\windows\a.bat syntax */ @@ -336,10 +338,14 @@ static BOOL search_path(const WCHAR *firstParam, WCHAR **full_path) } if (*pos) { /* Reached semicolon */ + if (pos - pathposn >= ARRAY_SIZE(thisDir)) + return FALSE; memcpy(thisDir, pathposn, (pos-pathposn) * sizeof(WCHAR)); thisDir[(pos-pathposn)] = 0x00; pathposn = pos+1; } else { /* Reached string end */ + if (lstrlenW(pathposn) >= ARRAY_SIZE(thisDir)) + return FALSE; lstrcpyW(thisDir, pathposn); pathposn = NULL; } @@ -356,7 +362,12 @@ static BOOL search_path(const WCHAR *firstParam, WCHAR **full_path) /* Since you can have eg. ..\.. on the path, need to expand to full information */ - GetFullPathNameW(temp, MAX_PATH, thisDir, NULL); + len = GetFullPathNameW(temp, ARRAY_SIZE(thisDir), thisDir, NULL); + if (!len || len >= ARRAY_SIZE(thisDir)) + return FALSE; + + if (lstrlenW(thisDir) + 1 + lstrlenW(stemofsearch) + 2 >= ARRAY_SIZE(thisDir)) + return FALSE; /* 1. If extension supplied, see if that file exists */ if (thisDir[lstrlenW(thisDir) - 1] != '\\') lstrcatW(thisDir, L"\\"); @@ -385,6 +396,13 @@ static BOOL search_path(const WCHAR *firstParam, WCHAR **full_path) /* 3. Yes - Try each path ext */ while (thisExt) { WCHAR *nextExt = wcschr(thisExt, ';'); + SIZE_T ext_len = nextExt ? nextExt - thisExt : lstrlenW(thisExt); + + if (pos + ext_len >= thisDir + ARRAY_SIZE(thisDir)) + { + thisExt = nextExt ? nextExt + 1 : NULL; + continue; + } if (nextExt) { memcpy(pos, thisExt, (nextExt-thisExt) * sizeof(WCHAR)); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/11387
On Fri Jul 24 07:40:59 2026 +0000, eric pouech wrote:
overall MR looks good I would have preferred to have the test in programs/cmd (as we're actually testing cmd's internal start) and written in batch language... is this something you could do? Done, thanks. I removed the standalone C test and added the regression test to programs/cmd/tests/test_builtins.cmd, with the equivalent test mirrored in test_builtins.bat.
The test registers a temporary URL protocol and verifies that cmd's built-in START can launch a URL longer than MAX_PATH. I also verified it on native Windows and with 32-bit and 64-bit Wine. The latest pipeline passes: https://gitlab.winehq.org/wine/wine/-/pipelines/71260 -- https://gitlab.winehq.org/wine/wine/-/merge_requests/11387#note_146575
This merge request was approved by Likun Li. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/11387
On Fri Jul 24 07:40:59 2026 +0000, Likun Li wrote:
Done, thanks. I removed the standalone C test and added the regression test to programs/cmd/tests/test_builtins.cmd, with the equivalent test mirrored in test_builtins.bat. The test registers a temporary URL protocol and verifies that cmd's built-in START can launch a URL longer than MAX_PATH. I also verified it on native Windows and with 32-bit and 64-bit Wine. The latest pipeline passes: https://gitlab.winehq.org/wine/wine/-/pipelines/71260 Hi, could you please confirm whether the current revision is ready to be merged? Thanks!
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/11387#note_149594
On Sat Aug 22 04:54:50 2026 +0000, Likun Li wrote:
Hi, could you please confirm whether the current revision is ready to be merged? Thanks! the test looks good
however, work has started to migrate start implementation inside cmd.exe (as it should be) (https://gitlab.winehq.org/wine/wine/-/merge_requests/11608) since the MR duplicates (and fixes) some code from cmd.exe, I think it's better not to change programs/start for now so perhaps the best way forward would be: * restrict this MR to the test only * then when !11608 lands, check if the test has been fixed or not; if not feel free to reinject your changes in cmd.exe * cc:ing @tati for visibility -- https://gitlab.winehq.org/wine/wine/-/merge_requests/11387#note_149703
participants (3)
-
eric pouech (＠epo) -
Likun Li (＠lilikun124)
-
lily11111222