From fdbd0a799c370d7fcf3832e04819c5ec9d85b6a7 Mon Sep 17 00:00:00 2001 From: Lakshmi Priya Sekar Date: Mon, 19 Oct 2015 11:01:24 -0700 Subject: [PATCH] Fix boundary bugs on buffer lengths. Include null char in MAX_LONGPATH. --- src/pal/src/file/file.cpp | 72 +++++++++++----------- src/pal/src/file/path.cpp | 40 +++++++----- src/pal/src/loader/module.cpp | 2 +- src/pal/src/misc/perftrace.cpp | 2 +- .../file_io/SearchPathA/test1/SearchPathA.c | 2 +- 5 files changed, 62 insertions(+), 56 deletions(-) diff --git a/src/pal/src/file/file.cpp b/src/pal/src/file/file.cpp index 36894dc..3f29c7f 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 a8fe5b3..953f1a2 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 +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 802b7e4..072d837 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 f2dd1fb..e2e65cb 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 7744f31..46cb57f 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; } -- 2.7.4