Fix boundary bugs on buffer lengths. Include null char in MAX_LONGPATH.

5 files changed, 62 insertions, 56 deletions

diff --git a/src/pal/src/file/file.cpp b/src/pal/src/file/file.cpp
index 36894dc7d2..3f29c7f519 100644
--- a/src/pal/src/file/file.cpp
+++ b/src/pal/src/file/file.cpp
@@ -45,7 +45,7 @@ using namespace CorUnix;
 
 SET_DEFAULT_DEBUG_CHANNEL(FILE);
 
-int MaxWCharToAcpLengthRatio = 3;
+int MaxWCharToAcpLengthFactor = 3;
 
 PAL_ERROR
 InternalSetFilePointerForUnixFd(
@@ -995,7 +995,7 @@ CreateFileW(
 
     if (lpFileName != NULL)
     {
-        length = (PAL_wcslen(lpFileName)+1) * MaxWCharToAcpLengthRatio;
+        length = (PAL_wcslen(lpFileName)+1) * MaxWCharToAcpLengthFactor;
     }
     
     name = namePathString.OpenStringBuffer(length);
@@ -1083,7 +1083,7 @@ CopyFileW(
     pThread = InternalGetCurrentThread();
     if (lpExistingFileName != NULL)
     {
-        length = (PAL_wcslen(lpExistingFileName)+1) * MaxWCharToAcpLengthRatio;
+        length = (PAL_wcslen(lpExistingFileName)+1) * MaxWCharToAcpLengthFactor;
     }
     
     source = sourcePathString.OpenStringBuffer(length);
@@ -1110,7 +1110,7 @@ CopyFileW(
     length = 0;
     if (lpNewFileName != NULL)
     {
-        length = (PAL_wcslen(lpNewFileName)+1) * MaxWCharToAcpLengthRatio;
+        length = (PAL_wcslen(lpNewFileName)+1) * MaxWCharToAcpLengthFactor;
     }
     
     dest = destPathString.OpenStringBuffer(length);
@@ -1286,7 +1286,7 @@ DeleteFileW(
     
     if (lpFileName != NULL)
     {
-        length = (PAL_wcslen(lpFileName)+1) * MaxWCharToAcpLengthRatio;
+        length = (PAL_wcslen(lpFileName)+1) * MaxWCharToAcpLengthFactor;
     }
     
     name = namePS.OpenStringBuffer(length);
@@ -1585,7 +1585,7 @@ MoveFileExW(
     
     if (lpExistingFileName != NULL)
     {
-        length = (PAL_wcslen(lpExistingFileName)+1) * MaxWCharToAcpLengthRatio;
+        length = (PAL_wcslen(lpExistingFileName)+1) * MaxWCharToAcpLengthFactor;
     }
     
     source = sourcePS.OpenStringBuffer(length);
@@ -1611,7 +1611,7 @@ MoveFileExW(
     length = 0;
     if (lpNewFileName != NULL)
     {
-        length = (PAL_wcslen(lpNewFileName)+1) * MaxWCharToAcpLengthRatio;
+        length = (PAL_wcslen(lpNewFileName)+1) * MaxWCharToAcpLengthFactor;
     }
     
     dest = destPS.OpenStringBuffer(length);
@@ -1776,7 +1776,7 @@ GetFileAttributesW(
         goto done;
     }
     
-    length = (PAL_wcslen(lpFileName)+1) * MaxWCharToAcpLengthRatio;
+    length = (PAL_wcslen(lpFileName)+1) * MaxWCharToAcpLengthFactor;
     filename = filenamePS.OpenStringBuffer(length);
     size = WideCharToMultiByte( CP_ACP, 0, lpFileName, -1, filename, length,
                                 NULL, NULL );
@@ -1856,7 +1856,7 @@ GetFileAttributesExW(
         goto done;
     }
     
-    length = (PAL_wcslen(lpFileName)+1) * MaxWCharToAcpLengthRatio;
+    length = (PAL_wcslen(lpFileName)+1) * MaxWCharToAcpLengthFactor;
     name = namePS.OpenStringBuffer(length);
     size = WideCharToMultiByte( CP_ACP, 0, lpFileName, -1, name, length,
                                 NULL, NULL );
@@ -1961,7 +1961,7 @@ SetFileAttributesW(
         goto done;
     }
     
-    length = (PAL_wcslen(lpFileName)+1) * MaxWCharToAcpLengthRatio;
+    length = (PAL_wcslen(lpFileName)+1) * MaxWCharToAcpLengthFactor;
     name = namePS.OpenStringBuffer(length);
     size = WideCharToMultiByte( CP_ACP, 0, lpFileName, -1, name, length,
                                 NULL, NULL );
@@ -3650,7 +3650,7 @@ GetTempFileNameW(
         goto done;
     }
 
-    length = (PAL_wcslen(lpPathName)+1) * MaxWCharToAcpLengthRatio;
+    length = (PAL_wcslen(lpPathName)+1) * MaxWCharToAcpLengthFactor;
     full_name = full_namePS.OpenStringBuffer(length);
     path_size = WideCharToMultiByte( CP_ACP, 0, lpPathName, -1, full_name,
                                      length, NULL, NULL );
@@ -3675,7 +3675,7 @@ GetTempFileNameW(
     
     if (lpPrefixString != NULL) 
     {
-        length = (PAL_wcslen(lpPrefixString)+1) * MaxWCharToAcpLengthRatio;
+        length = (PAL_wcslen(lpPrefixString)+1) * MaxWCharToAcpLengthFactor;
         prefix_string = prefix_stringPS.OpenStringBuffer(length);
         prefix_size = WideCharToMultiByte( CP_ACP, 0, lpPrefixString, -1, 
                                            prefix_string,
@@ -3701,7 +3701,7 @@ GetTempFileNameW(
         }
     }
     
-    tempfile_name = new char[MAX_LONGPATH+1];
+    tempfile_name = new char[MAX_LONGPATH];
     if (tempfile_name == NULL)
     {
         pThread->SetLastError(ERROR_NOT_ENOUGH_MEMORY);
@@ -3712,30 +3712,30 @@ GetTempFileNameW(
     uRet = GetTempFileNameA(full_name, 
                             (lpPrefixString == NULL) ? NULL : prefix_string,
                             0, tempfile_name);
-                        
-    path_size = MultiByteToWideChar( CP_ACP, 0, tempfile_name, -1, 
-                                       lpTempFileName, MAX_LONGPATH );
-
-    delete [] tempfile_name;
-    tempfile_name = NULL;
-    if ( uRet && !path_size)
-    {
-        DWORD dwLastError = GetLastError();
-        if (dwLastError == ERROR_INSUFFICIENT_BUFFER)
-        {
-            WARN("File names larger than MAX_PATH_FNAME (%d)! \n", MAX_LONGPATH);
-            dwLastError = ERROR_FILENAME_EXCED_RANGE;
-        }
-        else
+    if (uRet)
+    {                    
+        path_size = MultiByteToWideChar( CP_ACP, 0, tempfile_name, -1, 
+                                           lpTempFileName, MAX_LONGPATH );
+
+        delete [] tempfile_name;
+        tempfile_name = NULL;
+        if (!path_size)
         {
-            ASSERT("MultiByteToWideChar failure! error is %d", dwLastError);     
-            dwLastError = ERROR_INTERNAL_ERROR;
+            DWORD dwLastError = GetLastError();
+            if (dwLastError == ERROR_INSUFFICIENT_BUFFER)
+            {
+                WARN("File names larger than MAX_PATH_FNAME (%d)! \n", MAX_LONGPATH);
+                dwLastError = ERROR_FILENAME_EXCED_RANGE;
+            }
+            else
+            {
+                ASSERT("MultiByteToWideChar failure! error is %d", dwLastError);     
+                dwLastError = ERROR_INTERNAL_ERROR;
+            }
+            pThread->SetLastError(dwLastError);
+            uRet = 0;
         }
-        pThread->SetLastError(dwLastError);
-        uRet = 0;
-        goto done;
     }
-    
 
 done:
     LOGEXIT("GetTempFileNameW returns UINT %u\n", uRet);
@@ -4881,7 +4881,7 @@ Return value:
 BOOL FILEGetFileNameFromSymLink(char *source)
 {
     int ret;
-    char * sLinkData = new char[MAX_LONGPATH+1];
+    char * sLinkData = new char[MAX_LONGPATH];
 
     do
     {
@@ -4889,7 +4889,7 @@ BOOL FILEGetFileNameFromSymLink(char *source)
         if (ret>0)
         {
             sLinkData[ret] = '\0';
-            strcpy_s(source, sizeof(char)*(MAX_LONGPATH+1), sLinkData);
+            strcpy_s(source, sizeof(char)*(MAX_LONGPATH), sLinkData);
         }
     } while (ret > 0);
 
diff --git a/src/pal/src/file/path.cpp b/src/pal/src/file/path.cpp
index a8fe5b33da..953f1a2fa9 100644
--- a/src/pal/src/file/path.cpp
+++ b/src/pal/src/file/path.cpp
@@ -43,6 +43,7 @@ SET_DEFAULT_DEBUG_CHANNEL(FILE);
 // should be placed after the SET_DEFAULT_DEBUG_CHANNEL(FILE)
 #include <safemath.h>
 
+int MaxWCharToAcpLengthRatio = 3;
 /*++
 Function:
   GetFullPathNameA
@@ -248,7 +249,7 @@ GetFullPathNameW(
         goto done;
     }
     
-    bufferASize = MAX_LONGPATH * sizeof(WCHAR);
+    bufferASize = MAX_LONGPATH * MaxWCharToAcpLengthRatio;
     bufferA = bufferAPS.OpenStringBuffer(bufferASize);
     
     length = GetFullPathNameA(fileNameA, bufferASize, bufferA, &lpFilePartA);
@@ -1104,6 +1105,7 @@ SearchPathA(
     LPCSTR pPathEnd;
     size_t PathLength;
     size_t FileNameLength;
+    DWORD length;
     DWORD dw;
 
     PERF_ENTRY(SearchPathA);
@@ -1147,11 +1149,12 @@ SearchPathA(
             goto done;
         }
         /* Canonicalize the path to deal with back-to-back '/', etc. */
-        CanonicalFullPath = CanonicalFullPathPS.OpenStringBuffer(MAX_LONGPATH);
-        dw = GetFullPathNameA(lpFileName, MAX_LONGPATH, CanonicalFullPath, NULL);
+        length = MAX_LONGPATH;
+        CanonicalFullPath = CanonicalFullPathPS.OpenStringBuffer(length);
+        dw = GetFullPathNameA(lpFileName, length+1, CanonicalFullPath, NULL);
         CanonicalFullPathPS.CloseBuffer(dw);
         
-        if (dw > MAX_LONGPATH+1)
+        if (length+1 < dw)
         {
             CanonicalFullPath = CanonicalFullPathPS.OpenStringBuffer(dw-1);
             dw = GetFullPathNameA(lpFileName, dw,
@@ -1220,9 +1223,8 @@ SearchPathA(
             FullPath = FullPathPS.OpenStringBuffer(FullPathLength+1);
             memcpy(FullPath, pPathStart, PathLength);
             FullPath[PathLength] = '/';
-            if (strcpy_s(&FullPath[PathLength+1], MAX_LONGPATH-PathLength, lpFileName) != SAFECRT_SUCCESS)
+            if (strcpy_s(&FullPath[PathLength+1], FullPathLength+1-PathLength, lpFileName) != SAFECRT_SUCCESS)
             {
-                FullPathPS.CloseBuffer(0);
                 ERROR("strcpy_s failed!\n");
                 SetLastError( ERROR_FILENAME_EXCED_RANGE );
                 nRet = 0;
@@ -1231,12 +1233,13 @@ SearchPathA(
 
             FullPathPS.CloseBuffer(FullPathLength+1);            
             /* Canonicalize the path to deal with back-to-back '/', etc. */
-            CanonicalFullPath = CanonicalFullPathPS.OpenStringBuffer(MAX_LONGPATH);
-            dw = GetFullPathNameA(FullPath, MAX_LONGPATH,
+            length = MAX_LONGPATH;
+            CanonicalFullPath = CanonicalFullPathPS.OpenStringBuffer(length);
+            dw = GetFullPathNameA(FullPath, length+1,
                                   CanonicalFullPath, NULL);
             CanonicalFullPathPS.CloseBuffer(dw);
             
-            if (dw > MAX_LONGPATH+1)
+            if (length+1 < dw)
             {
                 CanonicalFullPath = CanonicalFullPathPS.OpenStringBuffer(dw-1);
                 dw = GetFullPathNameA(FullPath, dw,
@@ -1350,6 +1353,7 @@ SearchPathW(
     size_t PathLength;
     size_t FileNameLength;
     DWORD dw;
+    DWORD length;
     char * AnsiPath;
     PathCharString AnsiPathPS;
     size_t CanonicalPathLength;
@@ -1390,10 +1394,11 @@ SearchPathW(
     if('\\' == lpFileName[0] || '/' == lpFileName[0])
     {
         /* Canonicalize the path to deal with back-to-back '/', etc. */
-        CanonicalPath = CanonicalPathPS.OpenStringBuffer(MAX_LONGPATH);
-        dw = GetFullPathNameW(lpFileName, MAX_LONGPATH, CanonicalPath, NULL);
+        length = MAX_LONGPATH;
+        CanonicalPath = CanonicalPathPS.OpenStringBuffer(length);
+        dw = GetFullPathNameW(lpFileName, length+1, CanonicalPath, NULL);
         CanonicalPathPS.CloseBuffer(dw);
-        if (dw > MAX_LONGPATH+1)
+        if (length+1 < dw)
         {
             CanonicalPath = CanonicalPathPS.OpenStringBuffer(dw-1);
             dw = GetFullPathNameW(lpFileName, dw, CanonicalPath, NULL);
@@ -1409,7 +1414,7 @@ SearchPathW(
         }
 
         /* see if the file exists */
-        CanonicalPathLength = (PAL_wcslen(CanonicalPath)+1)*3;
+        CanonicalPathLength = (PAL_wcslen(CanonicalPath)+1) * MaxWCharToAcpLengthRatio;
         AnsiPath = AnsiPathPS.OpenStringBuffer(CanonicalPathLength);
 	    canonical_size = WideCharToMultiByte(CP_ACP, 0, CanonicalPath, -1,
 			    AnsiPath, CanonicalPathLength, NULL, NULL);
@@ -1474,12 +1479,13 @@ SearchPathW(
             FullPathPS.CloseBuffer(FullPathLength+1);
     
             /* Canonicalize the path to deal with back-to-back '/', etc. */
-            CanonicalPath = CanonicalPathPS.OpenStringBuffer(MAX_LONGPATH);
-            dw = GetFullPathNameW(FullPath, MAX_LONGPATH,
+            length = MAX_LONGPATH;
+            CanonicalPath = CanonicalPathPS.OpenStringBuffer(length);
+            dw = GetFullPathNameW(FullPath, length+1,
                                   CanonicalPath, NULL);
             CanonicalPathPS.CloseBuffer(dw);
             
-            if (dw > MAX_LONGPATH+1)
+            if (length+1 < dw)
             {
                 CanonicalPath = CanonicalPathPS.OpenStringBuffer(dw-1);
                 dw = GetFullPathNameW(FullPath, dw, CanonicalPath, NULL);
@@ -1495,7 +1501,7 @@ SearchPathW(
             }
     
             /* see if the file exists */
-            CanonicalPathLength = (PAL_wcslen(CanonicalPath)+1) * 3;
+            CanonicalPathLength = (PAL_wcslen(CanonicalPath)+1) * MaxWCharToAcpLengthRatio;
             AnsiPath = AnsiPathPS.OpenStringBuffer(CanonicalPathLength);
             canonical_size = WideCharToMultiByte(CP_ACP, 0, CanonicalPath, -1,
                                 AnsiPath, CanonicalPathLength, NULL, NULL);
diff --git a/src/pal/src/loader/module.cpp b/src/pal/src/loader/module.cpp
index 802b7e4711..072d837ebf 100644
--- a/src/pal/src/loader/module.cpp
+++ b/src/pal/src/loader/module.cpp
@@ -94,7 +94,7 @@ MODSTRUCT exe_module;
 MODSTRUCT *pal_module = NULL;
 
 char * g_szCoreCLRPath = NULL;
-size_t g_cbszCoreCLRPath = (MAX_LONGPATH+1) * sizeof(char);
+size_t g_cbszCoreCLRPath = MAX_LONGPATH * sizeof(char);
 
 int MaxWCharToAcpLength = 3;
 /* static function declarations ***********************************************/
diff --git a/src/pal/src/misc/perftrace.cpp b/src/pal/src/misc/perftrace.cpp
index f2dd1fb71e..e2e65cbec7 100644
--- a/src/pal/src/misc/perftrace.cpp
+++ b/src/pal/src/misc/perftrace.cpp
@@ -132,7 +132,7 @@ static double PERFComputeStandardDeviation(pal_perf_api_info *api);
 static void PERFPrintProgramHeaderInfo(PERF_FILE * hFile, BOOL completedExecution);
 static BOOL PERFInitProgramInfo(LPWSTR command_line, LPWSTR exe_path);
 static BOOL PERFReadSetting( );
-static void PERFLogFileName(PathCharString destFileString, const char *fileName, const char *suffix, int max_length);
+static void PERFLogFileName(PathCharString * destFileString, const char *fileName, const char *suffix, int max_length);
 static void PERFlushAllLogs();
 static int PERFWriteCounters(pal_perf_api_info * table); 
 static BOOL PERFFlushLog(pal_perf_thread_info * local_buffer, BOOL output_header);
diff --git a/src/pal/tests/palsuite/file_io/SearchPathA/test1/SearchPathA.c b/src/pal/tests/palsuite/file_io/SearchPathA/test1/SearchPathA.c
index 7744f31a99..46cb57fe15 100644
--- a/src/pal/tests/palsuite/file_io/SearchPathA/test1/SearchPathA.c
+++ b/src/pal/tests/palsuite/file_io/SearchPathA/test1/SearchPathA.c
@@ -139,7 +139,7 @@ int __cdecl main(int argc, char *argv[]) {
         Fail ("SearchPathA: ERROR2 -> Did not Find valid file[%s][%s][%d]\n", lpPath, szFileNameExistsWithExt, error);
     }
 
-    /*RemoveAll();*/
+    RemoveAll();
     PAL_Terminate();
     return PASS; 
 }