From: Haoyang Chen Subject: [PATCH v2] shell32: Avoid crashes caused by very long URLs. Message-Id: <20210528075548.17678-1-chenhaoyang@uniontech.com> Date: Fri, 28 May 2021 15:55:48 +0800 Signed-off-by: Haoyang Chen --- dlls/shell32/shlexec.c | 90 +++++++++++++++++++++--------------- dlls/shell32/tests/shlexec.c | 24 +++++++--- 2 files changed, 72 insertions(+), 42 deletions(-) diff --git a/dlls/shell32/shlexec.c b/dlls/shell32/shlexec.c index ce0b8f6d2be..1a1948b55a7 100644 --- a/dlls/shell32/shlexec.c +++ b/dlls/shell32/shlexec.c @@ -448,12 +448,17 @@ static BOOL SHELL_TryAppPathW( LPCWSTR szName, LPWSTR lpResult, WCHAR **env) '\\','C','u','r','r','e','n','t','V','e','r','s','i','o','n','\\','A','p','p',' ','P','a','t','h','s','\\',0}; static const WCHAR wPath[] = {'P','a','t','h',0}; HKEY hkApp = 0; - WCHAR buffer[1024]; + WCHAR *buffer; LONG len; LONG res; BOOL found = FALSE; + LONG bufferlen = lstrlenW(szName) + ARRAYSIZE(wszKeyAppPaths) + 1; if (env) *env = NULL; + if (!(buffer = heap_alloc(bufferlen * sizeof(WCHAR)))) + { + return FALSE; + } strcpyW(buffer, wszKeyAppPaths); strcatW(buffer, szName); res = RegOpenKeyExW(HKEY_LOCAL_MACHINE, buffer, 0, KEY_READ, &hkApp); @@ -466,13 +471,14 @@ static BOOL SHELL_TryAppPathW( LPCWSTR szName, LPWSTR lpResult, WCHAR **env) if (env) { - DWORD count = sizeof(buffer); + DWORD count = bufferlen * sizeof(WCHAR); if (!RegQueryValueExW(hkApp, wPath, NULL, NULL, (LPBYTE)buffer, &count) && buffer[0]) *env = SHELL_BuildEnvW( buffer ); } end: if (hkApp) RegCloseKey(hkApp); + if (buffer) heap_free(buffer); return found; } @@ -991,16 +997,23 @@ static UINT_PTR execute_from_key(LPCWSTR key, LPCWSTR lpFile, WCHAR *env, LPCWST { static const WCHAR wCommand[] = {'c','o','m','m','a','n','d',0}; static const WCHAR wDdeexec[] = {'d','d','e','e','x','e','c',0}; - WCHAR cmd[256], param[1024], ddeexec[256]; + WCHAR cmd[256], ddeexec[256]; + WCHAR *param; LONG cmdlen = sizeof(cmd), ddeexeclen = sizeof(ddeexec); UINT_PTR retval = SE_ERR_NOASSOC; DWORD resultLen; LPWSTR tmp; + LONG paramlen = cmdlen + lstrlenW(lpFile) + 1; TRACE("%s %s %s %s %s\n", debugstr_w(key), debugstr_w(lpFile), debugstr_w(env), debugstr_w(szCommandline), debugstr_w(executable_name)); cmd[0] = '\0'; + if (!(param = heap_alloc(paramlen * sizeof(WCHAR)))) + { + return (UINT_PTR)NULL; + } + param[0] = '\0'; /* Get the application from the registry */ @@ -1013,8 +1026,8 @@ static UINT_PTR execute_from_key(LPCWSTR key, LPCWSTR lpFile, WCHAR *env, LPCWST if (cmdlen >= ARRAY_SIZE(cmd)) cmdlen = ARRAY_SIZE(cmd) - 1; cmd[cmdlen] = '\0'; - SHELL_ArgifyW(param, ARRAY_SIZE(param), cmd, lpFile, psei->lpIDList, szCommandline, &resultLen); - if (resultLen > ARRAY_SIZE(param)) + SHELL_ArgifyW(param, paramlen, cmd, lpFile, psei->lpIDList, szCommandline, &resultLen); + if (resultLen > paramlen) ERR("Argify buffer not large enough, truncating\n"); } @@ -1038,6 +1051,7 @@ static UINT_PTR execute_from_key(LPCWSTR key, LPCWSTR lpFile, WCHAR *env, LPCWST else WARN("Nothing appropriate found for %s\n", debugstr_w(key)); + heap_free(param); return retval; } @@ -1588,11 +1602,11 @@ static BOOL SHELL_execute( LPSHELLEXECUTEINFOW sei, SHELL_ExecuteW32 execfunc ) SEE_MASK_CONNECTNETDRV | SEE_MASK_FLAG_DDEWAIT | SEE_MASK_UNICODE | SEE_MASK_ASYNCOK | SEE_MASK_HMONITOR; - WCHAR parametersBuffer[1024], dirBuffer[MAX_PATH], wcmdBuffer[1024]; - WCHAR *wszApplicationName, *wszParameters, *wszDir, *wcmd = NULL; + WCHAR dirBuffer[MAX_PATH]; + WCHAR *wcmdBuffer = NULL, *wszApplicationName, *wszParameters, *wszDir, *wcmd = NULL; DWORD dwApplicationNameLen = MAX_PATH+2; - DWORD parametersLen = ARRAY_SIZE(parametersBuffer); - DWORD wcmdLen = ARRAY_SIZE(wcmdBuffer); + DWORD parametersLen = 1024; + DWORD wcmdLen = 1024; DWORD len; SHELLEXECUTEINFOW sei_tmp; /* modifiable copy of SHELLEXECUTEINFO struct */ WCHAR *env; @@ -1633,19 +1647,21 @@ static BOOL SHELL_execute( LPSHELLEXECUTEINFOW sei, SHELL_ExecuteW32 execfunc ) memcpy(wszApplicationName, sei_tmp.lpFile, l*sizeof(WCHAR)); } - wszParameters = parametersBuffer; if (sei_tmp.lpParameters) { len = lstrlenW(sei_tmp.lpParameters) + 1; if (len > parametersLen) - { - wszParameters = heap_alloc(len * sizeof(WCHAR)); parametersLen = len; - } - strcpyW(wszParameters, sei_tmp.lpParameters); + wszParameters = heap_alloc(parametersLen * sizeof(WCHAR)); + strcpyW(wszParameters, sei_tmp.lpParameters); } else - *wszParameters = '\0'; + { + wszParameters = heap_alloc(parametersLen * sizeof(WCHAR)); + if (!wszParameters) + return FALSE; + *wszParameters = '\0'; + } wszDir = dirBuffer; if (sei_tmp.lpDirectory) @@ -1653,10 +1669,10 @@ static BOOL SHELL_execute( LPSHELLEXECUTEINFOW sei, SHELL_ExecuteW32 execfunc ) len = lstrlenW(sei_tmp.lpDirectory) + 1; if (len > ARRAY_SIZE(dirBuffer)) wszDir = heap_alloc(len * sizeof(WCHAR)); - strcpyW(wszDir, sei_tmp.lpDirectory); + strcpyW(wszDir, sei_tmp.lpDirectory); } else - *wszDir = '\0'; + *wszDir = '\0'; /* adjust string pointers to point to the new buffers */ sei_tmp.lpFile = wszApplicationName; @@ -1671,25 +1687,24 @@ static BOOL SHELL_execute( LPSHELLEXECUTEINFOW sei, SHELL_ExecuteW32 execfunc ) /* process the IDList */ if (sei_tmp.fMask & SEE_MASK_IDLIST) { - IShellExecuteHookW* pSEH; + IShellExecuteHookW* pSEH; - HRESULT hr = SHBindToParent(sei_tmp.lpIDList, &IID_IShellExecuteHookW, (LPVOID*)&pSEH, NULL); + HRESULT hr = SHBindToParent(sei_tmp.lpIDList, &IID_IShellExecuteHookW, (LPVOID*)&pSEH, NULL); - if (SUCCEEDED(hr)) - { - hr = IShellExecuteHookW_Execute(pSEH, &sei_tmp); + if (SUCCEEDED(hr)) + { + hr = IShellExecuteHookW_Execute(pSEH, &sei_tmp); - IShellExecuteHookW_Release(pSEH); + IShellExecuteHookW_Release(pSEH); - if (hr == S_OK) { + if (hr == S_OK) { heap_free(wszApplicationName); - if (wszParameters != parametersBuffer) - heap_free(wszParameters); + heap_free(wszParameters); if (wszDir != dirBuffer) heap_free(wszDir); - return TRUE; + return TRUE; } - } + } SHGetPathFromIDListW(sei_tmp.lpIDList, wszApplicationName); TRACE("-- idlist=%p (%s)\n", sei_tmp.lpIDList, debugstr_w(wszApplicationName)); @@ -1723,8 +1738,7 @@ static BOOL SHELL_execute( LPSHELLEXECUTEINFOW sei, SHELL_ExecuteW32 execfunc ) { sei->hInstApp = (HINSTANCE) 33; heap_free(wszApplicationName); - if (wszParameters != parametersBuffer) - heap_free(wszParameters); + heap_free(wszParameters); if (wszDir != dirBuffer) heap_free(wszDir); return TRUE; @@ -1737,8 +1751,7 @@ static BOOL SHELL_execute( LPSHELLEXECUTEINFOW sei, SHELL_ExecuteW32 execfunc ) if (retval <= 32 && !(sei_tmp.fMask & SEE_MASK_FLAG_NO_UI)) do_error_dialog(retval, sei_tmp.hwnd); heap_free(wszApplicationName); - if (wszParameters != parametersBuffer) - heap_free(wszParameters); + heap_free(wszParameters); if (wszDir != dirBuffer) heap_free(wszDir); return retval > 32; @@ -1805,6 +1818,11 @@ 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; + wcmdLen = (strlenW(wszApplicationName) > wcmdLen ? strlenW(wszApplicationName): wcmdLen) + 1; + wcmdBuffer = heap_alloc(wcmdLen * sizeof(WCHAR)); + if (!wcmdBuffer) + return FALSE; + wcmd = wcmdBuffer; strcpyW(wcmd, wszApplicationName); if (sei_tmp.lpDirectory) @@ -1820,12 +1838,12 @@ static BOOL SHELL_execute( LPSHELLEXECUTEINFOW sei, SHELL_ExecuteW32 execfunc ) sei, execfunc ); if (retval > 32) { heap_free(wszApplicationName); - if (wszParameters != parametersBuffer) - heap_free(wszParameters); + heap_free(wszParameters); if (wszDir != dirBuffer) heap_free(wszDir); if (wcmd != wcmdBuffer) heap_free(wcmd); + heap_free(wcmdBuffer); return TRUE; } @@ -1885,8 +1903,8 @@ end: TRACE("retval %lu\n", retval); heap_free(wszApplicationName); - if (wszParameters != parametersBuffer) - heap_free(wszParameters); + heap_free(wcmdBuffer); + heap_free(wszParameters); if (wszDir != dirBuffer) heap_free(wszDir); if (wcmd != wcmdBuffer) diff --git a/dlls/shell32/tests/shlexec.c b/dlls/shell32/tests/shlexec.c index 2481fb6faa2..d4f687bb0ad 100644 --- a/dlls/shell32/tests/shlexec.c +++ b/dlls/shell32/tests/shlexec.c @@ -66,7 +66,7 @@ static HANDLE dde_ready_event; static const char* encodeA(const char* str) { - static char encoded[2*1024+1]; + static char encoded[8*1024+1]; char* ptr; size_t len,i; @@ -94,7 +94,7 @@ static unsigned decode_char(char c) static char* decodeA(const char* str) { - static char decoded[1024]; + static char decoded[4096]; char* ptr; size_t len,i; @@ -115,7 +115,7 @@ static char* decodeA(const char* str) static void WINAPIV __WINE_PRINTF_ATTR(2,3) childPrintf(HANDLE h, const char* fmt, ...) { __ms_va_list valist; - char buffer[1024]; + char buffer[8192]; DWORD w; __ms_va_start(valist, fmt); @@ -126,7 +126,7 @@ static void WINAPIV __WINE_PRINTF_ATTR(2,3) childPrintf(HANDLE h, const char* fm static char* getChildString(const char* sect, const char* key) { - char buf[1024]; + char buf[8192]; char* ret; GetPrivateProfileStringA(sect, key, "-", buf, sizeof(buf), child_file); @@ -345,11 +345,11 @@ static void dump_child_(const char* file, int line) * ***/ -static char shell_call[2048]; +static char shell_call[4096]; static void WINAPIV __WINE_PRINTF_ATTR(2,3) _okShell(int condition, const char *msg, ...) { __ms_va_list valist; - char buffer[2048]; + char buffer[12288]; strcpy(buffer, shell_call); strcat(buffer, " "); @@ -1869,6 +1869,7 @@ static void test_fileurls(void) static void test_urls(void) { char url[MAX_PATH + 15]; + char long_url[2048]; INT_PTR rc; if (!create_test_class("fakeproto", FALSE)) @@ -1941,6 +1942,17 @@ static void test_urls(void) okChildString("argvA3", "URL"); okChildString("argvA4", "shlproto://foo/bar"); + memset(long_url, 0, sizeof(long_url)); + strcpy(long_url, "shlproto://foo/bar"); + memset(long_url + strlen(long_url), 'r', sizeof(long_url) - strlen(long_url) - 5); + strcat(long_url, ".exe"); + + rc = shell_execute(NULL, long_url, NULL, NULL); + ok(rc > 32, "%s failed: rc=%lu\n", shell_call, rc); + okChildInt("argcA", 5); + okChildString("argvA3", "URL"); + okChildString("argvA4", long_url); + /* Environment variables are expanded in URLs (but not in file URLs!) */ rc = shell_execute_ex(SEE_MASK_DOENVSUBST | SEE_MASK_FLAG_NO_UI, NULL, "shlproto://%TMPDIR%/bar", NULL, NULL, NULL); -- 2.20.1