Multi-argument system() invokes the shell under Win32

[p5pRT]

Migrated from rt.perl.org#43631 (status was 'open')

Searchable as RT43631$

[p5pRT]

From @pjf

G'day wonderful maintainers\,

I've discovered that under Win32 systems the multi-argument version of system() will invoke the shell if it cannot locate the command. The following code demonstrates the problem​:

  system("this_command_does_not_exist"\,1);   printf("raw​: %d ; apparent exit​: %d\n"\,$?\, $?>>8);

A warning ('this_command_does_not_exist' is not recognised as an internal or external command) is printed to STDERR as the shell fails to locate the command\, and $? is set to a value of 1 \<\< 8 (ie\, 256) which corresponds to the shell's exit value of 1\, meaning command not found.

As far as I can tell\, whenever the win32 port of Perl fails to start a command\, it tries it using the shell\, just in case that will succeed. This happens regardless of the form of system() used.

There are two issues this presents​:

  1) It's very difficult to tell the difference between a   call to system() that resulted in a command that   failed to start\, and a command that started successfully   but merely returned an exit value of 1.

  2) The shell may be invoked even though the multi-argument   form was used. This is not consistent with the   documentation in 'perldoc -f system' and 'perldoc -f exec'\,   which advertises this as a way to avoid the shell.

Having the multi-argument form of system always avoid the shell overcomes these difficulties\, and makes it consistent with the Unix port. This probably involves modification of the win32/win32.c functions Perl_do_aspawn() and do_spawn2()\, but it's been a long time since I've had to dig that far into Perl's guts.

Of course\, our risk here is that if anyone's actually been relying upon this behaviour then they're going to get quite a surprise. I'm not a Windows guru\, and can't easily gauge how much code may break from such a change.

I believe our other option is to document this in perlfunc\, perlport and perlwin32 appropriately.

I'm happy to do the legwork here (be it documentation or code)\, but I'm eager for advice as to what our best solution may be.

I'll be independently upgrading IPC​::System​::Simple on the CPAN in the next few days to provide an interface to system() that's consistent with the Unix implementation (always skipping the shell on multi-arg calls)\, as well as exposing the full 16-bit return value.

Cheerio\,

  Paul

Obligatory perl -V data follows.

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=define   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'\, lseeksize=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 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   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'

Characteristics of this binary (from libperl)​:   Compile-time options​: MULTIPLICITY PERL_IMPLICIT_CONTEXT   PERL_IMPLICIT_SYS PERL_MALLOC_WRAP   PL_OP_SLAB_ALLOC USE_ITHREADS USE_LARGE_FILES   USE_PERLIO USE_SITECUSTOMIZE   Locally applied patches​:   ActivePerl Build 817 [257965]   Iin_load_module moved for compatibility with build 806   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   Built under MSWin32   Compiled at Mar 20 2006 17​:54​:25   @​INC​:   C​:/Perl/lib   C​:/Perl/site/lib   .

[p5pRT]

From @pjf

G'day p5p\,

This is just a nudge on this bug\, since I potentially have some free cycles and can start taking actions to correct it. I'm also posting a simple example so you don't have to read the original bug report. This comes courtesy of http​://perlmonks.org/?node_id=625960 :

---cut here---

  system("echo"\, "%PATH%"); # works fine on Win32

And here's the crux of the problem. Currently\, that does work fine under Windows. According to the documention in perldoc -f exec\, it shouldn't​:

  Using an indirect object with "exec" or "system" is also more   secure. This usage (which also works fine with system()) forces   interpretation of the arguments as a multivalued list\, even if the   list had just one argu- ment. That way you're safe from the shell   expanding wildcards or splitting up words with whitespace in them.

  @​args = ( "echo surprise" ); exec @​args; # subject to shell escapes   # if @​args == 1

  exec { $args[0] } @​args; # safe even with one-arg list

Under Windows\, Perl's system() is currently doing exactly what it shouldn't do\, interpreting shell metacharacters and commands when called with multiple arguments. In my opinion\, this is a bug in Perl.

---cut here---

My fear here is that people may expecting multi-argument system under Windows to use the shell anyway\, and the previously referenced node on Perlmonks is a prime example of this.

Do we​:

1) Change multi-argument system() under Windows to never use the code. This may break existing deployed code\, and hence I feel is a Bad Idea.

2) Emit some sort of deprecation warning when multi-arg system() under Windows uses the shell and then move to (1). I think that this is an even worse idea\, because it's nigh impossible to tell when multi-arg system() is being called with the expectation of the shell getting involved.

3) Leave the code as-is\, and fix the documentation. This is easy\, and doesn't break existing systems. However it leaves system() behaving in an inconsistent fashion across platforms.

4) Same as (3)\, but provide some sort of mechanism to *make* them work consistently if desired\, preferably lexical in scope. For example​:

  use feature 'system';

Except probably with a less ambiguous name than just 'system'. This could be a no-op under Unix\, and actually do something useful under Windows. Except that last I checked system() and exec() take indirect arguments\, hence they can't be prototyped\, hence this is hard. It also provides no way of getting consistent behaviour with Perl \< 5.10.

5) Same a (3)\, but bundle a core module that people can use to make it consistent. Except this bloats the core\, and may be a no-op under Unix. It has the advantage that the code can be potentially distributed on the CPAN\, so it can be installed on older Perls if required.

Feedback and thoughts would be greatly appreciated. If you don't know\, or don't care\, then vote (3)\, as it's better than nothing.

Cheerio\,

  Paul

-- Paul Fenwick \pjf@&#8203;perltraining\.com\.au | http​://perltraining.com.au/ Director of Training | Ph​: +61 3 9354 6001 Perl Training Australia | Fax​: +61 3 9354 2681

[p5pRT]

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

[p5pRT]

From @pjf

(Reposting to p5p\, since I didn't realise that a simple response via RT wouldn't forward to p5p)

G'day p5p\,

This is just a nudge on this bug\, since I potentially have some free cycles and can start taking actions to correct it. I'm also posting a simple example so you don't have to read the original bug report. This comes courtesy of http​://perlmonks.org/?node_id=625960 :

---cut here---

  system("echo"\, "%PATH%"); # works fine on Win32

And here's the crux of the problem. Currently\, that does work fine under Windows. According to the documentation in perldoc -f exec\, it shouldn't​:

  Using an indirect object with "exec" or "system" is also more   secure. This usage (which also works fine with system()) forces   interpretation of the arguments as a multivalued list\, even if the   list had just one argu- ment. That way you're safe from the shell   expanding wildcards or splitting up words with whitespace in them.

  @​args = ( "echo surprise" ); exec @​args; # subject to shell escapes   # if @​args == 1

  exec { $args[0] } @​args; # safe even with one-arg list

Under Windows\, Perl's system() is currently doing exactly what it shouldn't do\, interpreting shell metacharacters and commands when called with multiple arguments. In my opinion\, this is a bug in Perl.

---cut here---

My fear here is that people may expecting multi-argument system under Windows to use the shell anyway\, and the previously referenced node on Perlmonks is a prime example of this.

Do we​:

1) Change multi-argument system() under Windows to never use the code. This may break existing deployed code\, and hence I feel is a Bad Idea.

2) Emit some sort of deprecation warning when multi-arg system() under Windows uses the shell and then move to (1). I think that this is an even worse idea\, because it's nigh impossible to tell when multi-arg system() is being called with the expectation of the shell getting involved.

3) Leave the code as-is\, and fix the documentation. This is easy\, and doesn't break existing systems. However it leaves system() behaving in an inconsistent fashion across platforms.

4) Same as (3)\, but provide some sort of mechanism to *make* them work consistently if desired\, preferably lexical in scope. For example​:

  use feature 'system';

Except probably with a less ambiguous name than just 'system'. This could be a no-op under Unix\, and actually do something useful under Windows. Except that last I checked system() and exec() take indirect arguments\, hence they can't be prototyped\, hence this is hard. It also provides no way of getting consistent behaviour with Perl \< 5.10.

5) Same a (3)\, but bundle a core module that people can use to make it consistent. Except this bloats the core\, and may be a no-op under Unix. It has the advantage that the code can be potentially distributed on the CPAN\, so it can be installed on older Perls if required.

Feedback and thoughts would be greatly appreciated. If you don't know\, or don't care\, then vote (3)\, as it's better than nothing.

Cheerio\,

  Paul

-- Paul Fenwick \pjf@&#8203;perltraining\.com\.au | http​://perltraining.com.au/ Director of Training | Ph​: +61 3 9354 6001 Perl Training Australia | Fax​: +61 3 9354 2681

[p5pRT]

From @demerphq

On 7/11/07\, Paul Fenwick \pjf@&#8203;perltraining\.com\.au wrote​:

1) Change multi-argument system() under Windows to never use the code. This may break existing deployed code\, and hence I feel is a Bad Idea.

If its done as part of a major upgrade its fair IMO.

But if Jan Dubois or Steve Hay were to disagree with me id bow to their opinion.

Yves

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

[p5pRT]

From @bulk88

On Wed Jul 11 06​:38​:37 2007\, demerphq wrote​:

But if Jan Dubois or Steve Hay were to disagree with me id bow to their opinion.

Yves

CCing them.

I played around with this bug a little bit\, when you feed a list to win32 system\, each scalar is double quoted. I couldn't cause any additional execution through using "&"s.

system('echo'\, ' & echo JAPH');

becomes

echo " & echo JAPH"

which fails\,so it is retried to CreateProcess as

cmd.exe /x/d/c "echo " & echo JAPH""

yes those 2 double quotes look funny.

The result printed to console is ______________________________________________ " & echo JAPH"

C​:\p517\perl> _______________________________________________ running manually on command line shows double execution ______________________________________________ C​:\p517\perl>echo & echo JAPH ECHO is on. JAPH

C​:\p517\perl> ______________________________________________ -- bulk88 ~ bulk88 at hotmail.com

[p5pRT]

From @jandubois

On Wed\, Dec 26\, 2012 at 5​:43 PM\, bulk88 via RT \perlbug\-followup@&#8203;perl\.orgwrote​:

On Wed Jul 11 06​:38​:37 2007\, demerphq wrote​:

But if Jan Dubois or Steve Hay were to disagree with me id bow to their opinion.

I agree with Yves. I think the current behavior is a bug\, and fixing it for a major new release like 5.18.0 or 5.20.0 should be acceptable.

Cheers\, -Jan

[toddr]

@demerphq @bulk88 We proposed a change for the next major version of perl. Was there a patch? did this ever get implemented?

[xenu]

This is still broken. I have an unfinished patch that fixes both this and #13190, but since it's a breaking change and we're almost in the user-visible changes freeze, I'll probably make a PR early in 5.34 release cycle.

[toddr]

Thanks! I have added a 5.34.0 milestone so we don't forget.