cmd: fix buffer overflow in WCMD_run_program

Eric Ho ericho921 at gmail.com
Thu Mar 4 11:20:30 CST 2010


Hi Guys,
 I'm a UCLA student working with Dan Kegel on cmd.
 This attached patch adds tests for the following buffer overflows, and
passes on winetestbot.  Fixes http://bugs.winehq.org/show_bug.cgi?id=21344.
Overflows fixed:
1. overflow due to long path name (unchecked memcpy,strcpy to thisDir)
2. overflow due to long file name (unchecked strcpy into stemofsearch)
3. overflow due to concatenating thisDir into stemofsearch (strcat)
4. added early breaks when getFullPathName returns an error
5. fixed incorrect errorlevel code (needs to be 9023 and it was 9009)
-Eric
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.winehq.org/pipermail/wine-devel/attachments/20100304/d1c70a80/attachment.htm>
-------------- next part --------------
diff --git a/programs/cmd/tests/test_builtins.cmd b/programs/cmd/tests/test_builtins.cmd
index ed2b75b..5a551fc 100644
--- a/programs/cmd/tests/test_builtins.cmd
+++ b/programs/cmd/tests/test_builtins.cmd
@@ -43,3 +43,24 @@ if /i foo==FOO echo if /i seems to work
 if /i not foo==FOO echo if /i seems to be broken
 if /I foo==FOO echo if /I seems to work
 if /I not foo==FOO echo if /I seems to be broken
+
+echo ------------ Testing overflow --------------
+xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx>NUL
+IF ERRORLEVEL 9023 echo Overflow by long filename with no explicit path was caught.
+IF NOT ERRORLEVEL 9023 echo Overflow by long filename with no explicit path was not caught
+
+./xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx>NUL
+IF ERRORLEVEL 9009 echo Overflow by long filename with explicit path was caught.
+IF NOT ERRORLEVEL 9009 echo Overflow by long filename with explicit path was not caught
+
+./xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/;./>NUL
+IF ERRORLEVEL 9009 echo Overflow by long path was caught.
+IF NOT ERRORLEVEL 9009 echo Overflow by long path was not caught
+
+./xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/;./>NUL
+IF ERRORLEVEL 9009 echo Overflow by stemofsearch concatenation was caught.
+IF NOT ERRORLEVEL 9009 echo Overflow stemofsearch concatenation was not caught
+
+./xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/xxxxxxxxxxxxxxxxxxxx>NUL
+IF ERRORLEVEL 9009 echo Overflow by allFiles concatenation was caught.
+IF NOT ERRORLEVEL 9009 echo Overflow allFiles concatenation was not caught
diff --git a/programs/cmd/tests/test_builtins.cmd.exp b/programs/cmd/tests/test_builtins.cmd.exp
index 2c0e980..f22b9c1 100644
--- a/programs/cmd/tests/test_builtins.cmd.exp
+++ b/programs/cmd/tests/test_builtins.cmd.exp
@@ -24,3 +24,9 @@ Testing case sensitivity with and without /i option
 if seems to default to case sensitivity
 if /i seems to work
 if /I seems to work
+------------ Testing overflow --------------
+Overflow by long filename with no explicit path was caught.
+Overflow by long filename with explicit path was caught.
+Overflow by long path was caught.
+Overflow by stemofsearch concatenation was caught.
+Overflow by allFiles concatenation was caught.
diff --git a/programs/cmd/wcmdmain.c b/programs/cmd/wcmdmain.c
old mode 100644
new mode 100755
index f97a1fd..2d82175
--- a/programs/cmd/wcmdmain.c
+++ b/programs/cmd/wcmdmain.c
@@ -995,6 +995,7 @@ void WCMD_run_program (WCHAR *command, int called) {
     if (strlenW(param1) >= MAX_PATH)
     {
         WCMD_output_asis(WCMD_LoadMessage(WCMD_LINETOOLONG));
+        errorlevel = 9023;
         return;
     }
 
@@ -1003,9 +1004,15 @@ void WCMD_run_program (WCHAR *command, int called) {
   } else {
 
     /* Convert eg. ..\fred to include a directory by removing file part */
-    GetFullPathNameW(param1, sizeof(pathtosearch)/sizeof(WCHAR), pathtosearch, NULL);
+    DWORD ret = GetFullPathNameW(param1, sizeof(pathtosearch)/sizeof(WCHAR), pathtosearch, NULL);
+    
+    if (ret == 0) goto run_program_file_not_found;
+
+    if (ret == sizeof(pathtosearch)/sizeof(WCHAR)) goto run_program_line_too_long;
+
     lastSlash = strrchrW(pathtosearch, '\\');
     if (lastSlash && strchrW(lastSlash, '.') != NULL) extensionsupplied = TRUE;
+    if (strlenW(lastSlash+1)>=MAX_PATH) goto run_program_line_too_long;
     strcpyW(stemofsearch, lastSlash+1);
 
     /* Reduce pathtosearch to a path with trailing '\' to support c:\a.bat and
@@ -1033,15 +1040,18 @@ void WCMD_run_program (WCHAR *command, int called) {
     WCHAR *pos               = NULL;
     BOOL  found             = FALSE;
     const WCHAR slashW[] = {'\\','\0'};
+    DWORD ret;
 
     /* Work on the first directory on the search path */
     pos = strchrW(pathposn, ';');
     if (pos) {
+      if (pos-pathposn >= MAX_PATH) goto run_program_line_too_long;
       memcpy(thisDir, pathposn, (pos-pathposn) * sizeof(WCHAR));
       thisDir[(pos-pathposn)] = 0x00;
       pathposn = pos+1;
 
     } else {
+      if (strlenW(pathposn)>=MAX_PATH) goto run_program_line_too_long;
       strcpyW(thisDir, pathposn);
       pathposn = NULL;
     }
@@ -1049,9 +1059,12 @@ void WCMD_run_program (WCHAR *command, int called) {
     /* Since you can have eg. ..\.. on the path, need to expand
        to full information                                      */
     strcpyW(temp, thisDir);
-    GetFullPathNameW(temp, MAX_PATH, thisDir, NULL);
+    ret = GetFullPathNameW(temp, MAX_PATH, thisDir, NULL);
+
+    if (ret==0) goto run_program_file_not_found;
+    if (ret == MAX_PATH||strlenW(slashW)+strlenW(thisDir)+strlenW(stemofsearch)>=MAX_PATH)
+      goto run_program_line_too_long;
 
-    /* 1. If extension supplied, see if that file exists */
     strcatW(thisDir, slashW);
     strcatW(thisDir, stemofsearch);
     pos = &thisDir[strlenW(thisDir)]; /* Pos = end of name */
@@ -1069,6 +1082,8 @@ void WCMD_run_program (WCHAR *command, int called) {
       WIN32_FIND_DATAW finddata;
       static const WCHAR allFiles[] = {'.','*','\0'};
 
+      if (strlenW(thisDir)+strlenW(allFiles) >= MAX_PATH) 
+        goto run_program_line_too_long;
       strcatW(thisDir,allFiles);
       h = FindFirstFileW(thisDir, &finddata);
       FindClose(h);
@@ -1081,10 +1096,14 @@ void WCMD_run_program (WCHAR *command, int called) {
           WCHAR *nextExt = strchrW(thisExt, ';');
 
           if (nextExt) {
+            if((pos-thisDir)+(nextExt-thisExt)>=MAX_PATH) 
+              goto run_program_line_too_long;
             memcpy(pos, thisExt, (nextExt-thisExt) * sizeof(WCHAR));
             pos[(nextExt-thisExt)] = 0x00;
             thisExt = nextExt+1;
           } else {
+	    if((pos-thisDir)+strlenW(thisExt)>=MAX_PATH) 
+              goto run_program_line_too_long;
             strcpyW(pos, thisExt);
             thisExt = NULL;
           }
@@ -1174,6 +1193,7 @@ void WCMD_run_program (WCHAR *command, int called) {
     }
   }
 
+ run_program_file_not_found:
   /* Not found anywhere - give up */
   SetLastError(ERROR_FILE_NOT_FOUND);
   WCMD_print_error ();
@@ -1183,6 +1203,10 @@ void WCMD_run_program (WCHAR *command, int called) {
   errorlevel = 9009;
   return;
 
+ run_program_line_too_long:
+  WCMD_output_asis(WCMD_LoadMessage(WCMD_LINETOOLONG));
+  errorlevel = 9009;
+  return;
 }
 
 /*****************************************************************************


More information about the wine-devel mailing list