[PATCH 1/2] programs/cmd: bail out when full path name exceeds MAX_PATH
Eric Pouech
eric.pouech at gmail.com
Fri Jan 21 03:32:59 CST 2022
this mimics native behavior and prevents buffer overflows
note: this is far from fixing all potential buffer overflows in builtin
cmd.exe, just a small step into the right direction
Signed-off-by: Eric Pouech <eric.pouech at gmail.com>
---
programs/cmd/batch.c | 2 +-
programs/cmd/builtins.c | 21 +++++++++++----------
programs/cmd/cmd.rc | 1 +
programs/cmd/directory.c | 2 +-
programs/cmd/wcmd.h | 2 ++
programs/cmd/wcmdmain.c | 18 +++++++++++++++---
6 files changed, 31 insertions(+), 15 deletions(-)
diff --git a/programs/cmd/batch.c b/programs/cmd/batch.c
index 2ff8e3ba9be..9a262c5fec5 100644
--- a/programs/cmd/batch.c
+++ b/programs/cmd/batch.c
@@ -472,7 +472,7 @@ void WCMD_HandleTildeModifiers(WCHAR **start, BOOL atExecute)
/* After this, we need full information on the file,
which is valid not to exist. */
if (!skipFileParsing) {
- if (GetFullPathNameW(outputparam, MAX_PATH, fullfilename, &filepart) == 0) {
+ if (!WCMD_get_fullpath(outputparam, MAX_PATH, fullfilename, &filepart)) {
exists = FALSE;
fullfilename[0] = 0x00;
} else {
diff --git a/programs/cmd/builtins.c b/programs/cmd/builtins.c
index d14e69e072a..2fc7e07f7aa 100644
--- a/programs/cmd/builtins.c
+++ b/programs/cmd/builtins.c
@@ -805,7 +805,7 @@ void WCMD_copy(WCHAR * args) {
WINE_TRACE("Destination supplied, processing to see if file or directory\n");
/* Convert to fully qualified path/filename */
- GetFullPathNameW(destination->name, ARRAY_SIZE(destname), destname, &filenamepart);
+ if (!WCMD_get_fullpath(destination->name, ARRAY_SIZE(destname), destname, &filenamepart)) return;
WINE_TRACE("Full dest name is '%s'\n", wine_dbgstr_w(destname));
/* If parameter is a directory, ensure it ends in \ */
@@ -887,7 +887,7 @@ void WCMD_copy(WCHAR * args) {
/* Convert to fully qualified path/filename in srcpath, file filenamepart pointing
to where the filename portion begins (used for wildcard expansion). */
- GetFullPathNameW(thiscopy->name, ARRAY_SIZE(srcpath), srcpath, &filenamepart);
+ if (!WCMD_get_fullpath(thiscopy->name, ARRAY_SIZE(srcpath), srcpath, &filenamepart)) return;
WINE_TRACE("Full src name is '%s'\n", wine_dbgstr_w(srcpath));
/* If parameter is a directory, ensure it ends in \* */
@@ -897,7 +897,7 @@ void WCMD_copy(WCHAR * args) {
/* We need to know where the filename part starts, so append * and
recalculate the full resulting path */
lstrcatW(thiscopy->name, L"*");
- GetFullPathNameW(thiscopy->name, ARRAY_SIZE(srcpath), srcpath, &filenamepart);
+ if (!WCMD_get_fullpath(thiscopy->name, ARRAY_SIZE(srcpath), srcpath, &filenamepart)) return;
WINE_TRACE("Directory, so full name is now '%s'\n", wine_dbgstr_w(srcpath));
} else if ((wcspbrk(srcpath, L"*?") == NULL) &&
@@ -907,7 +907,7 @@ void WCMD_copy(WCHAR * args) {
/* We need to know where the filename part starts, so append \* and
recalculate the full resulting path */
lstrcatW(thiscopy->name, L"\\*");
- GetFullPathNameW(thiscopy->name, ARRAY_SIZE(srcpath), srcpath, &filenamepart);
+ if (!WCMD_get_fullpath(thiscopy->name, ARRAY_SIZE(srcpath), srcpath, &filenamepart)) return;
WINE_TRACE("Directory, so full name is now '%s'\n", wine_dbgstr_w(srcpath));
}
@@ -1191,7 +1191,7 @@ static BOOL WCMD_delete_confirm_wildcard(const WCHAR *filename, BOOL *pPrompted)
WCHAR fpath[MAX_PATH];
/* Convert path into actual directory spec */
- GetFullPathNameW(filename, ARRAY_SIZE(fpath), fpath, NULL);
+ if (!WCMD_get_fullpath(filename, ARRAY_SIZE(fpath), fpath, NULL)) return FALSE;
_wsplitpath(fpath, drive, dir, fname, ext);
/* Only prompt for * and *.*, not *a, a*, *.a* etc */
@@ -1319,7 +1319,8 @@ static BOOL WCMD_delete_one (const WCHAR *thisArg) {
WCHAR ext[MAX_PATH];
/* Convert path into actual directory spec */
- GetFullPathNameW(argCopy, ARRAY_SIZE(thisDir), thisDir, NULL);
+ if (!WCMD_get_fullpath(argCopy, ARRAY_SIZE(thisDir), thisDir, NULL)) return FALSE;
+
_wsplitpath(thisDir, drive, dir, fname, ext);
lstrcpyW(thisDir, drive);
@@ -2933,8 +2934,8 @@ void WCMD_move (void)
/* If 2nd parm is directory, then use original filename */
/* Convert partial path to full path */
- GetFullPathNameW(param1, ARRAY_SIZE(input), input, NULL);
- GetFullPathNameW(param2, ARRAY_SIZE(output), output, NULL);
+ if (!WCMD_get_fullpath(param1, ARRAY_SIZE(input), input, NULL) ||
+ !WCMD_get_fullpath(param2, ARRAY_SIZE(output), output, NULL)) return;
WINE_TRACE("Move from '%s'('%s') to '%s'\n", wine_dbgstr_w(input),
wine_dbgstr_w(param1), wine_dbgstr_w(output));
@@ -3158,7 +3159,7 @@ void WCMD_rename (void)
}
/* Convert partial path to full path */
- GetFullPathNameW(param1, ARRAY_SIZE(input), input, NULL);
+ if (!WCMD_get_fullpath(param1, ARRAY_SIZE(input), input, NULL)) return;
WINE_TRACE("Rename from '%s'('%s') to '%s'\n", wine_dbgstr_w(input),
wine_dbgstr_w(param1), wine_dbgstr_w(param2));
dotDst = wcschr(param2, '.');
@@ -3439,7 +3440,7 @@ void WCMD_setshow_default (const WCHAR *args) {
WCHAR ext[MAX_PATH];
/* Convert path into actual directory spec */
- GetFullPathNameW(string, ARRAY_SIZE(fpath), fpath, NULL);
+ if (!WCMD_get_fullpath(string, ARRAY_SIZE(fpath), fpath, NULL)) return;
_wsplitpath(fpath, drive, dir, fname, ext);
/* Rebuild path */
diff --git a/programs/cmd/cmd.rc b/programs/cmd/cmd.rc
index fa604a6db56..f72ba87a9a2 100644
--- a/programs/cmd/cmd.rc
+++ b/programs/cmd/cmd.rc
@@ -406,4 +406,5 @@ Enter HELP <command> for further information on any of the above commands.\n"
WCMD_NOOPERATOR, "Expected an operator.\n"
WCMD_BADPAREN, "Mismatch in parentheses.\n"
WCMD_BADHEXOCT, "Badly formed number - must be one of decimal (12),\n hexadecimal (0x34) or octal (056).\n"
+ WCMD_FILENAMETOOLONG, "File name is too long.\n"
}
diff --git a/programs/cmd/directory.c b/programs/cmd/directory.c
index 24b18bfa81b..807f7b386b1 100644
--- a/programs/cmd/directory.c
+++ b/programs/cmd/directory.c
@@ -787,7 +787,7 @@ void WCMD_directory (WCHAR *args)
}
WINE_TRACE("Using location '%s'\n", wine_dbgstr_w(fullname));
- status = GetFullPathNameW(fullname, ARRAY_SIZE(path), path, NULL);
+ if (!WCMD_get_fullpath(fullname, ARRAY_SIZE(path), path, NULL)) continue;
/*
* If the path supplied does not include a wildcard, and the endpoint of the
diff --git a/programs/cmd/wcmd.h b/programs/cmd/wcmd.h
index a02395ca123..234c253b49a 100644
--- a/programs/cmd/wcmd.h
+++ b/programs/cmd/wcmd.h
@@ -71,6 +71,7 @@ void WCMD_endlocal (void);
void WCMD_enter_paged_mode(const WCHAR *);
void WCMD_exit (CMD_LIST **cmdList);
void WCMD_for (WCHAR *, CMD_LIST **cmdList);
+BOOL WCMD_get_fullpath(const WCHAR *, SIZE_T, WCHAR *, WCHAR **);
void WCMD_give_help (const WCHAR *args);
void WCMD_goto (CMD_LIST **cmdList);
void WCMD_if (WCHAR *, CMD_LIST **cmdList);
@@ -317,3 +318,4 @@ extern WCHAR version_string[];
#define WCMD_NOOPERATOR 1043
#define WCMD_BADPAREN 1044
#define WCMD_BADHEXOCT 1045
+#define WCMD_FILENAMETOOLONG 1046
diff --git a/programs/cmd/wcmdmain.c b/programs/cmd/wcmdmain.c
index 675d7c5f464..65601348b05 100644
--- a/programs/cmd/wcmdmain.c
+++ b/programs/cmd/wcmdmain.c
@@ -452,6 +452,18 @@ void WCMD_strsubstW(WCHAR *start, const WCHAR *next, const WCHAR *insert, int le
memcpy(start, insert, len * sizeof(*insert));
}
+BOOL WCMD_get_fullpath(const WCHAR* in, SIZE_T outsize, WCHAR* out, WCHAR** start)
+{
+ DWORD ret = GetFullPathNameW(in, outsize, out, start);
+ if (!ret) return FALSE;
+ if (ret > outsize)
+ {
+ WCMD_output_asis_stderr(WCMD_LoadMessage(WCMD_FILENAMETOOLONG));
+ return FALSE;
+ }
+ return TRUE;
+}
+
/***************************************************************************
* WCMD_skip_leading_spaces
*
@@ -1057,7 +1069,7 @@ void WCMD_run_program (WCHAR *command, BOOL called)
} else {
/* Convert eg. ..\fred to include a directory by removing file part */
- GetFullPathNameW(firstParam, ARRAY_SIZE(pathtosearch), pathtosearch, NULL);
+ if (!WCMD_get_fullpath(firstParam, ARRAY_SIZE(pathtosearch), pathtosearch, NULL)) return;
lastSlash = wcsrchr(pathtosearch, '\\');
if (lastSlash && wcschr(lastSlash, '.') != NULL) extensionsupplied = TRUE;
lstrcpyW(stemofsearch, lastSlash+1);
@@ -1125,7 +1137,7 @@ void WCMD_run_program (WCHAR *command, BOOL called)
/* Since you can have eg. ..\.. on the path, need to expand
to full information */
- GetFullPathNameW(temp, MAX_PATH, thisDir, NULL);
+ if (!WCMD_get_fullpath(temp, ARRAY_SIZE(thisDir), thisDir, NULL)) return;
}
/* 1. If extension supplied, see if that file exists */
@@ -2564,7 +2576,7 @@ int __cdecl wmain (int argc, WCHAR *argvW[])
WINE_TRACE("First parameter is '%s'\n", wine_dbgstr_w(thisArg));
if (wcschr(thisArg, '\\') != NULL) {
- GetFullPathNameW(thisArg, ARRAY_SIZE(string), string, NULL);
+ if (!WCMD_get_fullpath(thisArg, ARRAY_SIZE(string), string, NULL)) return FALSE;
WINE_TRACE("Full path name '%s'\n", wine_dbgstr_w(string));
p = string + lstrlenW(string);
More information about the wine-devel
mailing list