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

Yann Droneaud yann at droneaud.fr
Mon Mar 8 15:53:59 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.

Note: The length passed as parameter was the length of the expansion variable
to be checked. It is still used in the new version, allowing things like
%FOO%%BAR% to match %FOO% (btw, not tested).
---
 programs/cmd/wcmdmain.c |   19 +++++++++++--------
 1 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/programs/cmd/wcmdmain.c b/programs/cmd/wcmdmain.c
index b86801c..ab7ecbe 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
@@ -539,14 +542,14 @@ static WCHAR *WCMD_expand_envvar(WCHAR *start, WCHAR *forVar, WCHAR *forVal) {
         *(colonpos+1) = 0x00;
     }
 
-    WINE_TRACE("Retrieving contents of %s\n", wine_dbgstr_w(thisVar));
+    WINE_TRACE("Retrieving contents of '%s'\n", wine_dbgstr_w(thisVar));
 
     /* Expand to contents, if unchanged, return */
     /* Handle DATE, TIME, ERRORLEVEL and CD replacements allowing */
     /* override if existing env var called that name              */
     if ((CompareStringW(LOCALE_USER_DEFAULT,
                         NORM_IGNORECASE | SORT_STRINGSORT,
-                        thisVar, 12, ErrorLvlP, -1) == 2) &&
+                        thisVar, min(thisVarLen, WCMD_SCWLEN(ErrorLvlP)), 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, min(thisVarLen, WCMD_SCWLEN(DateP)), 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, min(thisVarLen, WCMD_SCWLEN(TimeP)), 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, min(thisVarLen, WCMD_SCWLEN(CdP)), 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, min(thisVarLen, WCMD_SCWLEN(RandomP)), 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