Skip to content

Commit

Permalink
win32: default the shell to cmd.exe in the Windows system directory
Browse files Browse the repository at this point in the history 
This prevents picking up cmd.exe from the current directory, or
even from the PATH.

This protects against a privilege escalation attack where an attacker
in a separate session creates a cmd.exe in a directory where the
target account happens to have its current directory.
  • Loading branch information
@tonycoz @leonerd
tonycoz authored and leonerd committed Nov 1, 2023
1 parent 7047915 commit 1747314
Show file tree
Hide file tree
Showing 2 changed files with 89 additions and 32 deletions.
30 changes: 18 additions & 12 deletions t/win32/system.t
View file
Expand Up @@ -82,6 +82,7 @@ close $F;
chdir($testdir);
END {
chdir($cwd) && rmtree("$cwd/$testdir") if -d "$cwd/$testdir";
unlink "cmd.exe";
}
if (open(my $EIN, "$cwd/win32/${exename}_exe.uu")) {
note "Unpacking $exename.exe";
Expand All @@ -104,21 +105,20 @@ else {
}
note "Compiling $exename.c";
note "$Config{cc} $Config{ccflags} $exename.c";
if (system("$Config{cc} $Config{ccflags} $minus_o $exename.c >log 2>&1") != 0) {
if (system("$Config{cc} $Config{ccflags} $minus_o $exename.c >log 2>&1") != 0 ||
!-f "$exename.exe") {
note "Could not compile $exename.c, status $?";
note "Where is your C compiler?";
skip_all "can't build test executable";
}
unless (-f "$exename.exe") {
if (open(LOG,'<log'))
{
while(<LOG>) {
note $_;
}
}
note "Where is your C compiler?";
if (open(LOG,'<log'))
{
while(<LOG>) {
note $_;
}
}
else {
warn "Cannot open log (in $testdir):$!";
warn "Cannot open log (in $testdir):$!";
}
skip_all "can't build test executable";
}
}
copy("$plxname.bat","$plxname.cmd");
Expand All @@ -128,6 +128,12 @@ unless (-x "$testdir/$exename.exe") {
skip_all "can't build test executable";
}

# test we only look for cmd.exe in the standard place
delete $ENV{PERLSHELL};
copy("$testdir/$exename.exe", "$testdir/cmd.exe") or die $!;
copy("$testdir/$exename.exe", "cmd.exe") or die $!;
$ENV{PATH} = qq("$testdir";$ENV{PATH});

open my $T, "$^X -I../lib -w win32/system_tests |"
or die "Can't spawn win32/system_tests: $!";
my $expect;
Expand Down
91 changes: 71 additions & 20 deletions win32/win32.c
View file
Expand Up @@ -72,7 +72,7 @@
int _CRT_glob = 0;
#endif

#if defined(__MINGW32__) && (__MINGW32_MAJOR_VERSION==1)
#if defined(__MINGW32__) && (__MINGW32_MAJOR_VERSION==1)
/* Mingw32-1.1 is missing some prototypes */
START_EXTERN_C
FILE * _wfopen(LPCWSTR wszFileName, LPCWSTR wszMode);
Expand Down Expand Up @@ -116,7 +116,7 @@ static char* win32_get_xlib(const char *pl,

static BOOL has_shell_metachars(const char *ptr);
static long tokenize(const char *str, char **dest, char ***destv);
static void get_shell(void);
static int get_shell(void);
static char* find_next_space(const char *s);
static int do_spawn2(pTHX_ const char *cmd, int exectype);
static int do_spawn2_handles(pTHX_ const char *cmd, int exectype,
Expand Down Expand Up @@ -204,9 +204,9 @@ set_silent_invalid_parameter_handler(BOOL newvalue)

static void
my_invalid_parameter_handler(const wchar_t* expression,
const wchar_t* function,
const wchar_t* file,
unsigned int line,
const wchar_t* function,
const wchar_t* file,
unsigned int line,
uintptr_t pReserved)
{
# ifdef _DEBUG
Expand Down Expand Up @@ -602,7 +602,13 @@ tokenize(const char *str, char **dest, char ***destv)
return items;
}

static void
static const char
cmd_opts[] = "/x/d/c";

static const char
shell_cmd[] = "cmd.exe";

static int
get_shell(void)
{
dTHX;
Expand All @@ -614,12 +620,53 @@ get_shell(void)
* interactive use (which is what most programs look in COMSPEC
* for).
*/
const char* defaultshell = "cmd.exe /x/d/c";
const char *usershell = PerlEnv_getenv("PERL5SHELL");
w32_perlshell_items = tokenize(usershell ? usershell : defaultshell,
&w32_perlshell_tokens,
&w32_perlshell_vec);
const char *shell = PerlEnv_getenv("PERL5SHELL");
if (shell) {
w32_perlshell_items = tokenize(shell,
&w32_perlshell_tokens,
&w32_perlshell_vec);
}
else {
/* tokenize does some Unix-ish like things like
\\ escaping that don't work well here
*/
char shellbuf[MAX_PATH];
UINT len = GetSystemDirectoryA(shellbuf, sizeof(shellbuf));
if (len == 0) {
translate_to_errno();
return -1;
}
else if (len >= MAX_PATH) {
/* buffer too small */
errno = E2BIG;
return -1;
}
if (shellbuf[len-1] != '\\') {
my_strlcat(shellbuf, "\\", sizeof(shellbuf));
++len;
}
if (len + sizeof(shell_cmd) > sizeof(shellbuf)) {
errno = E2BIG;
return -1;
}
my_strlcat(shellbuf, shell_cmd, sizeof(shellbuf));
len += sizeof(shell_cmd)-1;

Newx(w32_perlshell_vec, 3, char *);
Newx(w32_perlshell_tokens, len + 1 + sizeof(cmd_opts), char);

my_strlcpy(w32_perlshell_tokens, shellbuf, len+1);
my_strlcpy(w32_perlshell_tokens + len +1, cmd_opts,
sizeof(cmd_opts));

w32_perlshell_vec[0] = w32_perlshell_tokens;
w32_perlshell_vec[1] = w32_perlshell_tokens + len + 1;
w32_perlshell_vec[2] = NULL;

w32_perlshell_items = 2;
}
}
return 0;
}

int
Expand All @@ -637,7 +684,9 @@ Perl_do_aspawn(pTHX_ SV *really, SV **mark, SV **sp)
if (sp <= mark)
return -1;

get_shell();
if (get_shell() < 0)
return -1;

Newx(argv, (sp - mark) + w32_perlshell_items + 2, char*);

if (SvNIOKp(*(mark+1)) && !SvPOKp(*(mark+1))) {
Expand Down Expand Up @@ -767,7 +816,8 @@ do_spawn2_handles(pTHX_ const char *cmd, int exectype, const int *handles)
if (needToTry) {
char **argv;
int i = -1;
get_shell();
if (get_shell() < 0)
return -1;
Newx(argv, w32_perlshell_items + 2, char*);
while (++i < w32_perlshell_items)
argv[i] = w32_perlshell_vec[i];
Expand Down Expand Up @@ -2069,14 +2119,14 @@ win32_getenvironmentstrings(void)
}

/* Get the number of bytes required to store the ACP encoded string */
aenvstrings_len = WideCharToMultiByte(CP_ACP, WC_NO_BEST_FIT_CHARS,
aenvstrings_len = WideCharToMultiByte(CP_ACP, WC_NO_BEST_FIT_CHARS,
lpWStr, wenvstrings_len, NULL, 0, NULL, NULL);
lpTmp = lpStr = (char *)win32_calloc(aenvstrings_len, sizeof(char));
if(!lpTmp)
out_of_memory();

/* Convert the string from UTF-16 encoding to ACP encoding */
WideCharToMultiByte(CP_ACP, WC_NO_BEST_FIT_CHARS, lpWStr, wenvstrings_len, lpStr,
WideCharToMultiByte(CP_ACP, WC_NO_BEST_FIT_CHARS, lpWStr, wenvstrings_len, lpStr,
aenvstrings_len, NULL, NULL);

FreeEnvironmentStringsW(lpWStr);
Expand Down Expand Up @@ -2437,7 +2487,7 @@ win32_uname(struct utsname *name)
/* Timing related stuff */

int
do_raise(pTHX_ int sig)
do_raise(pTHX_ int sig)
{
if (sig < SIG_SIZE) {
Sighandler_t handler = w32_sighandler[sig];
Expand Down Expand Up @@ -2473,8 +2523,8 @@ void
sig_terminate(pTHX_ int sig)
{
Perl_warn(aTHX_ "Terminating on signal SIG%s(%d)\n",PL_sig_name[sig], sig);
/* exit() seems to be safe, my_exit() or die() is a problem in ^C
thread
/* exit() seems to be safe, my_exit() or die() is a problem in ^C
thread
*/
exit(sig);
}
Expand Down Expand Up @@ -2525,7 +2575,7 @@ win32_async_check(pTHX)
/* Above or other stuff may have set a signal flag */
if (PL_sig_pending)
despatch_signals();

return 1;
}

Expand Down Expand Up @@ -3268,7 +3318,8 @@ win32_pipe(int *pfd, unsigned int size, int mode)
DllExport PerlIO*
win32_popenlist(const char *mode, IV narg, SV **args)
{
get_shell();
if (get_shell() < 0)
return NULL;

return do_popen(mode, NULL, narg, args);
}
Expand Down

0 comments on commit 1747314

Please sign in to comment.