From @pjf

== Problem ==

The Windows build of Perl supports the PERL5SHELL environment
variable for setting the command that is invoked when
system() and qx() are used.

Unfortunately, Perl does NOT check PERL5SHELL when running
under taint mode. system() will execute without complaint
when a tainted PERL5SHELL environment variable is set,
even it will complain about other tainted environment
variables such as PATH.

== Impact ==

A user that can modify the environment of a Perl program
running under Windows can have it execute arbitrary code
if system() or qx() is used, even if the program is
running in taint mode.

== Example to reproduce ==

On a windows machine​:

  set PERL5SHELL=notepad

  perl -Te"$ENV{PATH}=q{c​:/windows/system32}; system(q{foo.txt});"

Notepad will successfully start on 'foo.txt', even though
$ENV{PERL5SHELL} contains tainted data.

== Solution ==

PERL5SHELL should be checked for tainted data in the same way
that PATH and other shell-influencing environment variables
are checked.

Flags:
     category=core
     severity=high

Site configuration information for perl v5.8.8:

Configured by builder at Mon Mar 20 17:54:00 2006.

Summary of my perl5 (revision 5 version 8 subversion 8) configuration:
   Platform:
     osname=MSWin32, osvers=5.0, archname=MSWin32-x86-multi-thread
     uname=''
     config_args='undef'
     hint=recommended, useposix=true, d_sigaction=undef
     usethreads=define use5005threads=undef useithreads=define 
usemultiplicity=de
fine
     useperlio=define d_sfio=undef uselargefiles=define usesocks=undef
     use64bitint=undef use64bitall=undef uselongdouble=undef
     usemymalloc=n, bincompat5005=undef
   Compiler:
     cc='cl', ccflags ='-nologo -GF -W3 -MD -Zi -DNDEBUG -O1 -DWIN32 
-D_CONSOLE -
DNO_STRICT -DHAVE_DES_FCRYPT -DNO_HASH_SEED -DUSE_SITECUSTOMIZE 
-DPERL_IMPLICIT_
CONTEXT -DPERL_IMPLICIT_SYS -DUSE_PERLIO -DPERL_MSVCRT_READFIX',
     optimize='-MD -Zi -DNDEBUG -O1',
     cppflags='-DWIN32'
     ccversion='12.00.8804', gccversion='', gccosandvers=''
     intsize=4, longsize=4, ptrsize=4, doublesize=8, byteorder=1234
     d_longlong=undef, longlongsize=8, d_longdbl=define, longdblsize=10
     ivtype='long', ivsize=4, nvtype='double', nvsize=8, 
Off_t='__int64', lseeksi
ze=8
     alignbytes=8, prototype=define
   Linker and Libraries:
     ld='link', ldflags ='-nologo -nodefaultlib -debug -opt:ref,icf 
-libpath:"C:
\Perl\lib\CORE"  -machine:x86'
     libpth=\lib
     libs=  oldnames.lib kernel32.lib user32.lib gdi32.lib winspool.lib 
  comdlg32
.lib advapi32.lib shell32.lib ole32.lib oleaut32.lib  netapi32.lib 
uuid.lib ws2_
32.lib mpr.lib winmm.lib  version.lib odbc32.lib odbccp32.lib msvcrt.lib
     perllibs=  oldnames.lib kernel32.lib user32.lib gdi32.lib 
winspool.lib  comd
lg32.lib advapi32.lib shell32.lib ole32.lib oleaut32.lib  netapi32.lib 
uuid.lib
ws2_32.lib mpr.lib winmm.lib  version.lib odbc32.lib odbccp32.lib msvcrt.lib
     libc=msvcrt.lib, so=dll, useshrplib=yes, libperl=perl58.lib
     gnulibc_version=''
   Dynamic Linking:
     dlsrc=dl_win32.xs, dlext=dll, d_dlsymun=undef, ccdlflags=' '
     cccdlflags=' ', lddlflags='-dll -nologo -nodefaultlib -debug 
-opt:ref,icf  -
libpath:"C:\Perl\lib\CORE"  -machine:x86'

Locally applied patches:
     ACTIVEPERL_LOCAL_PATCHES_ENTRY
     Iin_load_module moved for compatibility with build 806
     Avoid signal flag SA_RESTART for older versions of HP-UX
     PerlEx support in CGI::Carp
     Less verbose ExtUtils::Install and Pod::Find
     Patch for CAN-2005-0448 from Debian with modifications
     Partly reverted 24733 to preserve binary compatibilty
     27528 win32_pclose() error exit doesn't unlock mutex
     27527 win32_async_check() can loop indefinitely
     27515 ignore directories when searching @INC
     27359 Fix -d:Foo=bar syntax
     27210 Fix quote typo in c2ph
     27203 Allow compiling swigged C++ code
     27200 Make stat() on Windows handle trailing slashes correctly
     27194 Get perl_fini() running on HP-UX again
     27133 Initialise lastparen in the regexp structure
     27034 Avoid \"Prototype mismatch\" warnings with autouse
     26970 Make Passive mode the default for Net::FTP
     26921 Avoid getprotobyname/number calls in IO::Socket::INET
     26897,26903 Make common IPPROTO_* constants always available
     26670 Make '-s' on the shebang line parse -foo=bar switches
     26379 Fix alarm() for Windows 2003
     26087 Storable 0.1 compatibility
     25861 IO::File performace issue
     25084 long groups entry could cause memory exhaustion
     24699 ICMP_UNREACHABLE handling in Net::Ping


@INC for perl v5.8.8:
     C:/Perl/lib
     C:/Perl/site/lib
     .


Environment for perl v5.8.8:
     HOME (unset)
     LANG (unset)
     LANGUAGE (unset)
     LD_LIBRARY_PATH (unset)
     LOGDIR (unset)
 
PATH=C:\Perl\bin\;C:\WINDOWS\system32;C:\WINDOWS;C:\WINDOWS\System32\Wbem
     PERL_BADLANG (unset)
     SHELL (unset)


-- 
Paul Fenwick <pjf@perltraining.com.au> | http://perltraining.com.au/
Director of Training                   | Ph:  +61 3 9354 6001
Perl Training Australia                | Fax: +61 3 9354 2681

From @rgarcia

On 14/07/06, via RT Paul Fenwick <perlbug-followup@​perl.org> wrote​:

The Windows build of Perl supports the PERL5SHELL environment
variable for setting the command that is invoked when
system() and qx() are used.

Unfortunately, Perl does NOT check PERL5SHELL when running
under taint mode. system() will execute without complaint
when a tainted PERL5SHELL environment variable is set,
even it will complain about other tainted environment
variables such as PATH.

Could someone with a Windows machine available check if change #28591
in bleadperl fixes this hole ? Thanks.

The RT System itself - Status changed from 'new' to 'open'

From @demerphq

On 7/16/06, Rafael Garcia-Suarez <rgarciasuarez@​gmail.com> wrote​:

On 14/07/06, via RT Paul Fenwick <perlbug-followup@​perl.org> wrote​:

The Windows build of Perl supports the PERL5SHELL environment
variable for setting the command that is invoked when
system() and qx() are used.

Unfortunately, Perl does NOT check PERL5SHELL when running
under taint mode. system() will execute without complaint
when a tainted PERL5SHELL environment variable is set,
even it will complain about other tainted environment
variables such as PATH.

Could someone with a Windows machine available check if change #28591
in bleadperl fixes this hole ? Thanks.

Unfortunately not. :-(

BTW, to test this on a normal Win2k build one needs to say

set PERL5SHELL=notepad
perl -Te"$ENV{PATH}=q{c​:/winnt/system32}; system(q{foo.txt});"

I think the original

set PERL5SHELL=notepad
perl -Te"$ENV{PATH}=q{c​:/windows/system32}; system(q{foo.txt});"

Should work fine on a normal XP or 9x build.

But as of \28591 the problem remains. Notepad is still launched.

Sorry. :-(

--
perl -Mre=debug -e "/just|another|perl|hacker/"

From @demerphq

On 7/17/06, demerphq <demerphq@​gmail.com> wrote​:

On 7/16/06, Rafael Garcia-Suarez <rgarciasuarez@​gmail.com> wrote​:

On 14/07/06, via RT Paul Fenwick <perlbug-followup@​perl.org> wrote​:

The Windows build of Perl supports the PERL5SHELL environment
variable for setting the command that is invoked when
system() and qx() are used.

Unfortunately, Perl does NOT check PERL5SHELL when running
under taint mode. system() will execute without complaint
when a tainted PERL5SHELL environment variable is set,
even it will complain about other tainted environment
variables such as PATH.

Could someone with a Windows machine available check if change #28591
in bleadperl fixes this hole ? Thanks.

Unfortunately not. :-(

BTW, to test this on a normal Win2k build one needs to say

set PERL5SHELL=notepad
perl -Te"$ENV{PATH}=q{c​:/winnt/system32}; system(q{foo.txt});"

I think the original

set PERL5SHELL=notepad
perl -Te"$ENV{PATH}=q{c​:/windows/system32}; system(q{foo.txt});"

Should work fine on a normal XP or 9x build.

But as of \28591 the problem remains. Notepad is still launched.

Sorry. :-(

Whoops. It helps to use the correct perl to do the tests.

D​:\dev\perl\ver\28591_\win32>..\perl
-Te"$ENV{PATH}=q{c​:/winnt/system32}; system(q{foo.txt});"
Insecure $ENV{PERL5SHELL} while running with -T switch at -e line 1.

So all is good, sorry for the confusion. *blush*

Yves
--
perl -Mre=debug -e "/just|another|perl|hacker/"

From @druud62

demerphq schreef​:

BTW, to test this on a normal Win2k build one needs to say

set PERL5SHELL=notepad
perl -Te"$ENV{PATH}=q{c​:/winnt/system32}; system(q{foo.txt});"

I think the original

set PERL5SHELL=notepad
perl -Te"$ENV{PATH}=q{c​:/windows/system32}; system(q{foo.txt});"

Should work fine on a normal XP or 9x build.

See also $ENV{SystemRoot} and $ENV{windir}.

--
Affijn, Ruud

"Gewoon is een tijger."

From @rgarcia

On 17/07/06, Dr.Ruud <rvtol+news@​isolution.nl> wrote​:

See also $ENV{SystemRoot} and $ENV{windir}.

What do they do ? If they are judged security-sensitive, they should
be added (the same way I added PERL5SHELL to taint.c in that patch.)

From @demerphq

On 7/17/06, Rafael Garcia-Suarez <rgarciasuarez@​gmail.com> wrote​:

On 17/07/06, Dr.Ruud <rvtol+news@​isolution.nl> wrote​:

See also $ENV{SystemRoot} and $ENV{windir}.

What do they do ? If they are judged security-sensitive, they should
be added (the same way I added PERL5SHELL to taint.c in that patch.)

These are one of the simpler ways to find where the various system
directories are on a given windows box. An alternative way is to use
the Win32 API to get the value of the relevent "special folder".

Generally people assume that things will be in C​:\Windows\System32
(Win9x compatible AND XP, and maybe later builds?) or
C​:\Winnt\System32 (NT4, Win2k), but in fact there is no reason they
couldnt be something else. For instance ive seen people who for
"security reasons" install their system into totally different
locations.

I dont know that they are security sensitive. Of course I dont know
that they arent either.

Yves

--
perl -Mre=debug -e "/just|another|perl|hacker/"

From @druud62

"Rafael Garcia-Suarez" schreef​:

Dr.Ruud​:

See also $ENV{SystemRoot} and $ENV{windir}.

What do they do ?

They both hold the value "C​:\WINNT" on a Win2k-system here. On other
Windows-systems they are "C​:\Windows".

I only mentioned them because Yves pointed to a difference between Win2k
and XP/9x​:

  set PERL5SHELL=notepad
  perl -Te"$ENV{PATH}=$ENV{windir}.q{/system32}; system(q{foo.txt});"

If they are judged security-sensitive, they should
be added (the same way I added PERL5SHELL to taint.c in that patch.)

I don't know how security-sensitive they are.

$ grep -iwrl 'SystemRoot' *
README.win32
lib/Module/Build/Platform/Windows.pm
t/win32/longpath.t
t/win32/system.t
win32/bin/pl2bat.pl
win32/ext/Win32API/File/t/file.t
win32/makefile.mk

$ grep -iwrl 'windir' *
lib/Module/Build/Base.pm
lib/Module/Build.pm
win32/ext/Win32/Win32.pm
win32/ext/Win32API/File/t/file.t
win32/makefile.mk

--
Affijn, Ruud

"Gewoon is een tijger."

From @jandubois

On Mon, 17 Jul 2006, Dr.Ruud wrote​:

If they are judged security-sensitive, they should be added (the
same way I added PERL5SHELL to taint.c in that patch.)

I don't know how security-sensitive they are.

$ grep -iwrl 'SystemRoot' *
README.win32
lib/Module/Build/Platform/Windows.pm
t/win32/longpath.t
t/win32/system.t
win32/bin/pl2bat.pl
win32/ext/Win32API/File/t/file.t
win32/makefile.mk

$ grep -iwrl 'windir' *
lib/Module/Build/Base.pm
lib/Module/Build.pm
win32/ext/Win32/Win32.pm
win32/ext/Win32API/File/t/file.t
win32/makefile.mk

Those references are all benign as far as I can tell.

Cheers,
-Jan

From guest@guest.guest.xxxxxxxx

G'day Affijn and P5P,

Firstly, let me thank you all once again for a very swift bugfix and for
all your hard work in maintaining Perl.

I also want to note that I'm a bit of a weenie when it comes to Windows
security, so take everything I say with a big grain of salt.

On Mon Jul 17 14​:58​:14 2006, rvtol+news@​isolution.nl wrote​:

See also $ENV{SystemRoot} and $ENV{windir}.

If they are judged security-sensitive, they should
be added (the same way I added PERL5SHELL to taint.c in that patch.)

I don't know how security-sensitive they are.

I feel that $ENV{SystemRoot} and $ENV{windir} carry a similar risk to
$ENV{PATH} when you've specified an absolute path to your executable.
To quote perlsec​:

  [Since] Perl can't guarantee that the executable in question isn't
  itself going to turn around and execute some other program
  that is dependent on your PATH, it makes sure you set the PATH.

However unlike PATH which the developer can set to a known-good value, I
don't believe we can do the same with SystemRoot/windir. As already
noted early in this discussion, the system administrator may have
legitimately put these directories just about anywhere, so we HAVE to
ask the operating system if we want to be sure these are good values.

Asking the OS appears to involve something like this​:

  use Win32 qw(CSIDL_WINDOWS);

  $ENV{windir} = Win32​::GetFolderPath(CSIDL_WINDOWS);

assuming that Win32's GetFolderPath is authoritative.

The other downside of making Perl check for windir/SystemRoot
taintedness is that it's likely to break every single taint-mode program
running in Windows land. That's unlikely to be popular.

I think there may be some value in raising an exception if we're running
on windows, in taint mode, that windir/SystemRoot is set to a tainted
value, AND this value does not match the system supplied values. This
gives us protection against environment attacks while not breaking
existing programs. Unfortunately it's not a trivial change.

All the very best,

  Paul

p5p@spam.wizbit.be - Status changed from 'open' to 'resolved'