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@​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@​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@​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@​perl.org>wrote​:

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.