[PATCH v2 0/2] MR17: programs/cmd: test that read is successful before using its result

[eric pouech (＠epo)]

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

[PATCH v2 1/2] programs/cmd: test that read is successful before using its result

[Eric Pouech]

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

[PATCH v2 2/2] programs/cmd: skip too long paths

[Eric Pouech]

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