[PATCH] cmd: fixed use of uninitialised memory in WCMD_expand_envvar() (valgrind). (try 2)

Yann Droneaud yann at droneaud.fr
Sat Mar 13 07:03:41 CST 2010


WCMD_expand_envvar() was using incorrectly CompareStringW(),
giving it a length bigger than the token stored in thisVar,
so CompareStringW() was reading uninitialised memory past the NUL
character stored in thisVar.

Since previous patch, min() macro calls were removed as proved
unnecessary by tests (in patches) :
 kernel32: added some basic tests for CompareString().
 cmd: added some expansion tests. (try 2)
---
 programs/cmd/wcmdmain.c |   17 ++++++++++-------
 1 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/programs/cmd/wcmdmain.c b/programs/cmd/wcmdmain.c
index b2243e3..85a41ce 100644
--- a/programs/cmd/wcmdmain.c
+++ b/programs/cmd/wcmdmain.c
@@ -485,8 +485,10 @@ static WCHAR *WCMD_expand_envvar(WCHAR *start, WCHAR *forVar, WCHAR *forVal) {
     WCHAR thisVar[MAXSTRING];
     WCHAR thisVarContents[MAXSTRING];
     WCHAR savedchar = 0x00;
+    size_t thisVarLen;
     int len;
 
+#define WCMD_SCWLEN(name) ((sizeof(name)/sizeof(name[0])) - 1)
     static const WCHAR ErrorLvl[]  = {'E','R','R','O','R','L','E','V','E','L','\0'};
     static const WCHAR ErrorLvlP[] = {'%','E','R','R','O','R','L','E','V','E','L','%','\0'};
     static const WCHAR Date[]      = {'D','A','T','E','\0'};
@@ -527,8 +529,9 @@ static WCHAR *WCMD_expand_envvar(WCHAR *start, WCHAR *forVar, WCHAR *forVal) {
         if (endOfVar2 != NULL) endOfVar = endOfVar2;
     }
 
-    memcpy(thisVar, start, ((endOfVar - start) + 1) * sizeof(WCHAR));
-    thisVar[(endOfVar - start)+1] = 0x00;
+    thisVarLen = (endOfVar - start) + 1;
+    memcpy(thisVar, start, thisVarLen * sizeof(WCHAR));
+    thisVar[thisVarLen] = 0x00;
     colonpos = strchrW(thisVar+1, ':');
 
     /* If there's complex substitution, just need %var% for now
@@ -546,7 +549,7 @@ static WCHAR *WCMD_expand_envvar(WCHAR *start, WCHAR *forVar, WCHAR *forVal) {
     /* override if existing env var called that name              */
     if ((CompareStringW(LOCALE_USER_DEFAULT,
                         NORM_IGNORECASE | SORT_STRINGSORT,
-                        thisVar, 12, ErrorLvlP, -1) == 2) &&
+                        thisVar, thisVarLen, ErrorLvlP, WCMD_SCWLEN(ErrorLvlP) ) == 2) &&
                 (GetEnvironmentVariableW(ErrorLvl, thisVarContents, 1) == 0) &&
                 (GetLastError() == ERROR_ENVVAR_NOT_FOUND)) {
       static const WCHAR fmt[] = {'%','d','\0'};
@@ -555,7 +558,7 @@ static WCHAR *WCMD_expand_envvar(WCHAR *start, WCHAR *forVar, WCHAR *forVal) {
 
     } else if ((CompareStringW(LOCALE_USER_DEFAULT,
                                NORM_IGNORECASE | SORT_STRINGSORT,
-                               thisVar, 6, DateP, -1) == 2) &&
+                               thisVar, thisVarLen, DateP, WCMD_SCWLEN(DateP) ) == 2) &&
                 (GetEnvironmentVariableW(Date, thisVarContents, 1) == 0) &&
                 (GetLastError() == ERROR_ENVVAR_NOT_FOUND)) {
 
@@ -565,7 +568,7 @@ static WCHAR *WCMD_expand_envvar(WCHAR *start, WCHAR *forVar, WCHAR *forVal) {
 
     } else if ((CompareStringW(LOCALE_USER_DEFAULT,
                                NORM_IGNORECASE | SORT_STRINGSORT,
-                               thisVar, 6, TimeP, -1) == 2) &&
+                               thisVar, thisVarLen, TimeP, WCMD_SCWLEN(TimeP) ) == 2) &&
                 (GetEnvironmentVariableW(Time, thisVarContents, 1) == 0) &&
                 (GetLastError() == ERROR_ENVVAR_NOT_FOUND)) {
       GetTimeFormatW(LOCALE_USER_DEFAULT, TIME_NOSECONDS, NULL,
@@ -574,7 +577,7 @@ static WCHAR *WCMD_expand_envvar(WCHAR *start, WCHAR *forVar, WCHAR *forVal) {
 
     } else if ((CompareStringW(LOCALE_USER_DEFAULT,
                                NORM_IGNORECASE | SORT_STRINGSORT,
-                               thisVar, 4, CdP, -1) == 2) &&
+                               thisVar, thisVarLen, CdP, WCMD_SCWLEN(CdP) ) == 2) &&
                 (GetEnvironmentVariableW(Cd, thisVarContents, 1) == 0) &&
                 (GetLastError() == ERROR_ENVVAR_NOT_FOUND)) {
       GetCurrentDirectoryW(MAXSTRING, thisVarContents);
@@ -582,7 +585,7 @@ static WCHAR *WCMD_expand_envvar(WCHAR *start, WCHAR *forVar, WCHAR *forVal) {
 
     } else if ((CompareStringW(LOCALE_USER_DEFAULT,
                                NORM_IGNORECASE | SORT_STRINGSORT,
-                               thisVar, 8, RandomP, -1) == 2) &&
+                               thisVar, thisVarLen, RandomP, WCMD_SCWLEN(RandomP) ) == 2) &&
                 (GetEnvironmentVariableW(Random, thisVarContents, 1) == 0) &&
                 (GetLastError() == ERROR_ENVVAR_NOT_FOUND)) {
       static const WCHAR fmt[] = {'%','d','\0'};
-- 
1.6.2.5




More information about the wine-patches mailing list