shlwapi: fix the handling of overflows in PathCombine[AW]
Mikołaj Zalewski
mikolaj at zalewski.pl
Mon Apr 23 13:03:52 CDT 2007
When an overflow happens in PathCombine[AW] the function should return
NULL and the output should be set to an empty string.
-------------- next part --------------
From 9bf7c7852b2242b8c7c23d8b72048750ff494ae6 Mon Sep 17 00:00:00 2001
From: =?utf-8?q?Miko=C5=82aj_Zalewski?= <mikolaj at zalewski.pl>
Date: Thu, 5 Apr 2007 23:14:42 +0200
Subject: [PATCH] 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..4945043 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());
}
--
1.4.4.2
More information about the wine-patches
mailing list