[PATCH v2 0/2] MR17: programs/cmd: test that read is successful before using its result
There are cases where the read can fail (not attached to a console, input stream mapped to /dev/null...) Signed-off-by: Eric Pouech <eric.pouech(a)gmail.com> -- v2: programs/cmd: skip too long paths https://gitlab.winehq.org/wine/wine/-/merge_requests/17
From: Eric Pouech <eric.pouech(a)gmail.com> There are cases where the read can fail (not attached to a console, input stream mapped to /dev/null...) Signed-off-by: Eric Pouech <eric.pouech(a)gmail.com> --- programs/cmd/builtins.c | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/programs/cmd/builtins.c b/programs/cmd/builtins.c index 963a9eaf361..dd3ebf76d50 100644 --- a/programs/cmd/builtins.c +++ b/programs/cmd/builtins.c @@ -193,7 +193,8 @@ static BOOL WCMD_ask_confirm (const WCHAR *message, BOOL showSureText, if (showSureText) WCMD_output_asis (confirm); WCMD_output_asis (options); - WCMD_ReadFile(GetStdHandle(STD_INPUT_HANDLE), answer, ARRAY_SIZE(answer), &count); + if (!WCMD_ReadFile(GetStdHandle(STD_INPUT_HANDLE), answer, ARRAY_SIZE(answer), &count)) + return FALSE; answer[0] = towupper(answer[0]); if (answer[0] == Ybuffer[0]) return TRUE; @@ -383,7 +384,12 @@ void WCMD_choice (const WCHAR * args) { /* FIXME: Add support for option /T */ answer[1] = 0; /* terminate single character string */ - WCMD_ReadFile(GetStdHandle(STD_INPUT_HANDLE), answer, 1, &count); + if (!WCMD_ReadFile(GetStdHandle(STD_INPUT_HANDLE), answer, 1, &count)) + { + heap_free(my_command); + errorlevel = 0; + return; + } if (!opt_s) answer[0] = towupper(answer[0]); @@ -3506,8 +3512,8 @@ void WCMD_setshow_date (void) { WCMD_output (WCMD_LoadMessage(WCMD_CURRENTDATE), curdate); if (wcsstr(quals, L"/T") == NULL) { WCMD_output (WCMD_LoadMessage(WCMD_NEWDATE)); - WCMD_ReadFile(GetStdHandle(STD_INPUT_HANDLE), buffer, ARRAY_SIZE(buffer), &count); - if (count > 2) { + if (WCMD_ReadFile(GetStdHandle(STD_INPUT_HANDLE), buffer, ARRAY_SIZE(buffer), &count) && + count > 2) { WCMD_output_stderr (WCMD_LoadMessage(WCMD_NYI)); } } @@ -4142,8 +4148,7 @@ void WCMD_setshow_env (WCHAR *s) { if (*p) WCMD_output_asis(p); /* Read the reply */ - WCMD_ReadFile(GetStdHandle(STD_INPUT_HANDLE), string, ARRAY_SIZE(string), &count); - if (count > 1) { + if (WCMD_ReadFile(GetStdHandle(STD_INPUT_HANDLE), string, ARRAY_SIZE(string), &count) && count > 1) { string[count-1] = '\0'; /* ReadFile output is not null-terminated! */ if (string[count-2] == '\r') string[count-2] = '\0'; /* Under Windoze we get CRLF! */ WINE_TRACE("set /p: Setting var '%s' to '%s'\n", wine_dbgstr_w(s), @@ -4295,8 +4300,8 @@ void WCMD_setshow_time (void) { WCMD_output (WCMD_LoadMessage(WCMD_CURRENTTIME), curtime); if (wcsstr(quals, L"/T") == NULL) { WCMD_output (WCMD_LoadMessage(WCMD_NEWTIME)); - WCMD_ReadFile(GetStdHandle(STD_INPUT_HANDLE), buffer, ARRAY_SIZE(buffer), &count); - if (count > 2) { + if (WCMD_ReadFile(GetStdHandle(STD_INPUT_HANDLE), buffer, ARRAY_SIZE(buffer), &count) && + count > 2) { WCMD_output_stderr (WCMD_LoadMessage(WCMD_NYI)); } } @@ -4717,8 +4722,8 @@ int WCMD_volume(BOOL set_label, const WCHAR *path) HIWORD(serial), LOWORD(serial)); if (set_label) { WCMD_output (WCMD_LoadMessage(WCMD_VOLUMEPROMPT)); - WCMD_ReadFile(GetStdHandle(STD_INPUT_HANDLE), string, ARRAY_SIZE(string), &count); - if (count > 1) { + if (WCMD_ReadFile(GetStdHandle(STD_INPUT_HANDLE), string, ARRAY_SIZE(string), &count) && + count > 1) { string[count-1] = '\0'; /* ReadFile output is not null-terminated! */ if (string[count-2] == '\r') string[count-2] = '\0'; /* Under Windoze we get CRLF! */ } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/17
From: Eric Pouech <eric.pouech(a)gmail.com> v2: protect against overflow in trace with debugstr_w Signed-off-by: Eric Pouech <eric.pouech(a)gmail.com> --- programs/cmd/builtins.c | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/programs/cmd/builtins.c b/programs/cmd/builtins.c index dd3ebf76d50..dd3ae5b509b 100644 --- a/programs/cmd/builtins.c +++ b/programs/cmd/builtins.c @@ -1355,6 +1355,13 @@ static BOOL WCMD_delete_one (const WCHAR *thisArg) { DIRECTORY_STACK *nextDir; WCHAR subParm[MAX_PATH]; + if (wcslen(thisDir) + wcslen(fd.cFileName) + 1 + wcslen(fname) + wcslen(ext) >= MAX_PATH) + { + WINE_TRACE("Skipping path too long %s%s\\%s%s\n", + debugstr_w(thisDir), debugstr_w(fd.cFileName), + debugstr_w(fname), debugstr_w(ext)); + continue; + } /* Work out search parameter in sub dir */ lstrcpyW (subParm, thisDir); lstrcatW (subParm, fd.cFileName); @@ -1761,7 +1768,14 @@ static void WCMD_add_dirstowalk(DIRECTORY_STACK *dirsToWalk) { (lstrcmpW(fd.cFileName, L"..") != 0) && (lstrcmpW(fd.cFileName, L".") != 0)) { /* Allocate memory, add to list */ - DIRECTORY_STACK *toWalk = heap_xalloc(sizeof(DIRECTORY_STACK)); + DIRECTORY_STACK *toWalk; + if (wcslen(dirsToWalk->dirName) + 1 + wcslen(fd.cFileName) >= MAX_PATH) + { + WINE_TRACE("Skipping too long path %s\\%s\n", + debugstr_w(dirsToWalk->dirName), debugstr_w(fd.cFileName)); + continue; + } + toWalk = heap_xalloc(sizeof(DIRECTORY_STACK)); WINE_TRACE("(%p->%p)\n", remainingDirs, remainingDirs->next); toWalk->next = remainingDirs->next; remainingDirs->next = toWalk; @@ -2321,6 +2335,12 @@ void WCMD_for (WCHAR *p, CMD_LIST **cmdList) { WINE_TRACE("Processing FOR filename %s\n", wine_dbgstr_w(fd.cFileName)); if (doRecurse) { + if (wcslen(dirsToWalk->dirName) + 1 + wcslen(fd.cFileName) >= MAX_PATH) + { + WINE_TRACE("Skipping too long path %s\\%s\n", + debugstr_w(dirsToWalk->dirName), debugstr_w(fd.cFileName)); + continue; + } lstrcpyW(fullitem, dirsToWalk->dirName); lstrcatW(fullitem, L"\\"); lstrcatW(fullitem, fd.cFileName); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/17
participants (2)
-
Eric Pouech -
eric pouech (＠epo)