Mikołaj Zalewski : shlwapi: Fix the handling of overflows in PathCombine[AW].
Module: wine Branch: master Commit: 7370a93b52c1f753fa0e305b688f2c8003b80afa URL: http://source.winehq.org/git/wine.git/?a=commit;h=7370a93b52c1f753fa0e305b68... Author: Mikołaj Zalewski <mikolaj(a)zalewski.pl> Date: Thu Apr 5 23:14:42 2007 +0200 shlwapi: Fix the handling of overflows in PathCombine[AW]. --- dlls/shlwapi/path.c | 51 ++++++++++++++++++---------- dlls/shlwapi/tests/path.c | 80 +++++++++++++++++++++++---------------------- 2 files changed, 74 insertions(+), 57 deletions(-) diff --git a/dlls/shlwapi/path.c b/dlls/shlwapi/path.c index a06198f..0ccda95 100644 --- a/dlls/shlwapi/path.c +++ b/dlls/shlwapi/path.c @@ -131,23 +131,31 @@ BOOL WINAPI PathAppendW(LPWSTR lpszPath, LPCWSTR lpszAppend) */ LPSTR WINAPI PathCombineA(LPSTR lpszDest, LPCSTR lpszDir, LPCSTR lpszFile) { + WCHAR szDest[MAX_PATH]; + WCHAR szDir[MAX_PATH]; + WCHAR szFile[MAX_PATH]; TRACE("(%p,%s,%s)\n", lpszDest, debugstr_a(lpszDir), debugstr_a(lpszFile)); - if (!lpszDest || (!lpszDir && !lpszFile)) - return NULL; /* Invalid parameters */ - else + /* Invalid parameters */ + if (!lpszDest) + return NULL; + if (!lpszDir && !lpszFile) { - WCHAR szDest[MAX_PATH]; - WCHAR szDir[MAX_PATH]; - WCHAR szFile[MAX_PATH]; - if (lpszDir) - MultiByteToWideChar(CP_ACP,0,lpszDir,-1,szDir,MAX_PATH); - if (lpszFile) - MultiByteToWideChar(CP_ACP,0,lpszFile,-1,szFile,MAX_PATH); - PathCombineW(szDest, lpszDir ? szDir : NULL, lpszFile ? szFile : NULL); - WideCharToMultiByte(CP_ACP,0,szDest,-1,lpszDest,MAX_PATH,0,0); + lpszDest[0] = 0; + return NULL; } - return lpszDest; + + if (lpszDir) + MultiByteToWideChar(CP_ACP,0,lpszDir,-1,szDir,MAX_PATH); + if (lpszFile) + MultiByteToWideChar(CP_ACP,0,lpszFile,-1,szFile,MAX_PATH); + + if (PathCombineW(szDest, lpszDir ? szDir : NULL, lpszFile ? szFile : NULL)) + if (WideCharToMultiByte(CP_ACP,0,szDest,-1,lpszDest,MAX_PATH,0,0)) + return lpszDest; + + lpszDest[0] = 0; + return NULL; } /************************************************************************* @@ -162,8 +170,14 @@ LPWSTR WINAPI PathCombineW(LPWSTR lpszDest, LPCWSTR lpszDir, LPCWSTR lpszFile) TRACE("(%p,%s,%s)\n", lpszDest, debugstr_w(lpszDir), debugstr_w(lpszFile)); - if (!lpszDest || (!lpszDir && !lpszFile)) - return NULL; /* Invalid parameters */ + /* Invalid parameters */ + if (!lpszDest) + return NULL; + if (!lpszDir && !lpszFile) + { + lpszDest[0] = 0; + return NULL; + } if ((!lpszFile || !*lpszFile) && lpszDir) { @@ -194,10 +208,11 @@ LPWSTR WINAPI PathCombineW(LPWSTR lpszDest, LPCWSTR lpszDir, LPCWSTR lpszFile) PathStripToRootW(szTemp); lpszFile++; /* Skip '\' */ } - if (!PathAddBackslashW(szTemp)) - return NULL; - if (strlenW(szTemp) + strlenW(lpszFile) >= MAX_PATH) + if (!PathAddBackslashW(szTemp) || strlenW(szTemp) + strlenW(lpszFile) >= MAX_PATH) + { + lpszDest[0] = 0; return NULL; + } strcatW(szTemp, lpszFile); } diff --git a/dlls/shlwapi/tests/path.c b/dlls/shlwapi/tests/path.c index c91e99f..6f52ac7 100644 --- a/dlls/shlwapi/tests/path.c +++ b/dlls/shlwapi/tests/path.c @@ -952,6 +952,9 @@ static void test_PathMatchSpec(void) static void test_PathCombineW(void) { LPWSTR wszString, wszString2; + WCHAR wbuf[MAX_PATH+1], wstr1[MAX_PATH] = {'C',':','\\',0}, wstr2[MAX_PATH]; + static const WCHAR expout[] = {'C',':','\\','A','A',0}; + int i; wszString2 = HeapAlloc(GetProcessHeap(), 0, MAX_PATH * sizeof(WCHAR)); @@ -960,12 +963,32 @@ static void test_PathCombineW(void) ok (wszString == NULL, "Expected a NULL return\n"); /* Some NULL */ + wszString2[0] = 'a'; wszString = pPathCombineW(wszString2, NULL, NULL); ok (wszString == NULL, "Expected a NULL return\n"); - + ok (wszString2[0] == 0, "Destination string not empty\n"); + HeapFree(GetProcessHeap(), 0, wszString2); + + /* overflow test */ + wstr2[0] = wstr2[1] = wstr2[2] = 'A'; + for (i=3; i<MAX_PATH/2; i++) + wstr1[i] = wstr2[i] = 'A'; + wstr1[(MAX_PATH/2) - 1] = wstr2[MAX_PATH/2] = 0; + memset(wbuf, 0xbf, sizeof(wbuf)); + + wszString = pPathCombineW(wbuf, wstr1, wstr2); + ok(wszString == NULL, "Expected a NULL return\n"); + ok(wbuf[0] == 0, "Buffer contains data\n"); + + /* PathCombineW can be used in place */ + wstr1[3] = 0; + wstr2[2] = 0; + ok(PathCombineW(wstr1, wstr1, wstr2) == wstr1, "Expected a wstr1 return\n"); + ok(StrCmpW(wstr1, expout) == 0, "Unexpected PathCombine output\n"); } + #define LONG_LEN (MAX_PATH * 2) #define HALF_LEN (MAX_PATH / 2 + 1) @@ -1039,10 +1062,7 @@ static void test_PathCombineA(void) lstrcpyA(dest, "control"); str = PathCombineA(dest, NULL, NULL); ok(str == NULL, "Expected str == NULL, got %p\n", str); - todo_wine - { - ok(lstrlenA(dest) == 0, "Expected 0 length, got %i\n", lstrlenA(dest)); - } + ok(lstrlenA(dest) == 0, "Expected 0 length, got %i\n", lstrlenA(dest)); ok(GetLastError() == 0xdeadbeef, "Expected 0xdeadbeef, got %d\n", GetLastError()); /* try directory without backslash */ @@ -1125,23 +1145,17 @@ static void test_PathCombineA(void) SetLastError(0xdeadbeef); lstrcpyA(dest, "control"); str = PathCombineA(dest, "C:\\", too_long); - todo_wine - { - ok(str == NULL, "Expected str == NULL, got %p\n", str); - ok(lstrlenA(dest) == 0, "Expected 0 length, got %i\n", lstrlenA(dest)); - ok(GetLastError() == 0xdeadbeef, "Expected 0xdeadbeef, got %d\n", GetLastError()); - } + ok(str == NULL, "Expected str == NULL, got %p\n", str); + ok(lstrlenA(dest) == 0, "Expected 0 length, got %i\n", lstrlenA(dest)); + todo_wine ok(GetLastError() == 0xdeadbeef, "Expected 0xdeadbeef, got %d\n", GetLastError()); /* try a directory longer than MAX_PATH */ SetLastError(0xdeadbeef); lstrcpyA(dest, "control"); str = PathCombineA(dest, too_long, "one\\two\\three"); - todo_wine - { - ok(str == NULL, "Expected str == NULL, got %p\n", str); - ok(lstrlenA(dest) == 0, "Expected 0 length, got %i\n", lstrlenA(dest)); - ok(GetLastError() == 0xdeadbeef, "Expected 0xdeadbeef, got %d\n", GetLastError()); - } + ok(str == NULL, "Expected str == NULL, got %p\n", str); + ok(lstrlenA(dest) == 0, "Expected 0 length, got %i\n", lstrlenA(dest)); + todo_wine ok(GetLastError() == 0xdeadbeef, "Expected 0xdeadbeef, got %d\n", GetLastError()); memset(one, 'b', HALF_LEN); memset(two, 'c', HALF_LEN); @@ -1152,11 +1166,8 @@ static void test_PathCombineA(void) SetLastError(0xdeadbeef); lstrcpyA(dest, "control"); str = PathCombineA(dest, one, two); - todo_wine - { - ok(str == NULL, "Expected str == NULL, got %p\n", str); - ok(lstrlenA(dest) == 0, "Expected 0 length, got %i\n", lstrlenA(dest)); - } + ok(str == NULL, "Expected str == NULL, got %p\n", str); + ok(lstrlenA(dest) == 0, "Expected 0 length, got %i\n", lstrlenA(dest)); ok(GetLastError() == 0xdeadbeef, "Expected 0xdeadbeef, got %d\n", GetLastError()); } @@ -1320,12 +1331,9 @@ static void test_PathAppendA(void) too_long[LONG_LEN - 1] = '\0'; SetLastError(0xdeadbeef); res = PathAppendA(too_long, "two\\three"); - todo_wine - { - ok(!res, "Expected failure\n"); - ok(GetLastError() == 0xdeadbeef, "Expected 0xdeadbeef, got %d\n", GetLastError()); - ok(lstrlen(too_long) == 0, "Expected length of too_long to be zero, got %i\n", lstrlen(too_long)); - } + ok(!res, "Expected failure\n"); + todo_wine ok(GetLastError() == 0xdeadbeef, "Expected 0xdeadbeef, got %d\n", GetLastError()); + ok(lstrlen(too_long) == 0, "Expected length of too_long to be zero, got %i\n", lstrlen(too_long)); /* pszMore is too long */ lstrcpy(path, "C:\\one"); @@ -1333,12 +1341,9 @@ static void test_PathAppendA(void) too_long[LONG_LEN - 1] = '\0'; SetLastError(0xdeadbeef); res = PathAppendA(path, too_long); - todo_wine - { - ok(!res, "Expected failure\n"); - ok(GetLastError() == 0xdeadbeef, "Expected 0xdeadbeef, got %d\n", GetLastError()); - ok(lstrlen(path) == 0, "Expected length of path to be zero, got %i\n", lstrlen(path)); - } + ok(!res, "Expected failure\n"); + todo_wine ok(GetLastError() == 0xdeadbeef, "Expected 0xdeadbeef, got %d\n", GetLastError()); + ok(lstrlen(path) == 0, "Expected length of path to be zero, got %i\n", lstrlen(path)); /* both params combined are too long */ memset(one, 'a', HALF_LEN); @@ -1347,11 +1352,8 @@ static void test_PathAppendA(void) two[HALF_LEN - 1] = '\0'; SetLastError(0xdeadbeef); res = PathAppendA(one, two); - todo_wine - { - ok(!res, "Expected failure\n"); - ok(lstrlen(one) == 0, "Expected length of one to be zero, got %i\n", lstrlen(one)); - } + ok(!res, "Expected failure\n"); + ok(lstrlen(one) == 0, "Expected length of one to be zero, got %i\n", lstrlen(one)); ok(GetLastError() == 0xdeadbeef, "Expected 0xdeadbeef, got %d\n", GetLastError()); }
participants (1)
-
Alexandre Julliard