Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[PATCH] Document that "exec LIST" and "system LIST" may fall back to the shell on Win32 #13907

Closed
p5pRT opened this issue Jun 5, 2014 · 17 comments

Comments

@p5pRT
Copy link
Collaborator

@p5pRT p5pRT commented Jun 5, 2014

Migrated from rt.perl.org#122046 (status was 'resolved')

Searchable as RT122046$

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jun 5, 2014

From tsibley@cpan.org

Patch against blead attached.

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jun 5, 2014

From tsibley@cpan.org

0001-Document-that-exec-LIST-and-system-LIST-may-fall-bac.patch
From d84365fb13141367bf8d63cb8c14094c35a30688 Mon Sep 17 00:00:00 2001
From: Thomas Sibley <tsibley@cpan.org>
Date: Thu, 5 Jun 2014 10:17:42 -0700
Subject: [PATCH] Document that "exec LIST" and "system LIST" may fall back to
 the shell on Win32

As noted on p5p [1] and subsequently discussed [2].

win32.c contains the following fall back in Perl_do_aspawn():

    status = win32_spawnvp(flag,
                           (const char*)(really ? SvPV_nolen(really) : argv[0]),
                           (const char* const*)argv);

    if (status < 0 && (errno == ENOEXEC || errno == ENOENT)) {
        /* possible shell-builtin, invoke with shell */
        int sh_items;
        sh_items = w32_perlshell_items;
        while (--index >= 0)
            argv[index+sh_items] = argv[index];
        while (--sh_items >= 0)
            argv[sh_items] = w32_perlshell_vec[sh_items];

        status = win32_spawnvp(flag,
                               (const char*)(really ? SvPV_nolen(really) : argv[0]),
                               (const char* const*)argv);
    }

[1] http://www.nntp.perl.org/group/perl.perl5.porters/2014/04/msg214556.html
[2] http://www.nntp.perl.org/group/perl.perl5.porters/2014/04/msg214564.html
---
 pod/perlfunc.pod | 14 ++++++++++----
 pod/perlport.pod |  6 ++++++
 2 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/pod/perlfunc.pod b/pod/perlfunc.pod
index aff2cd5..ad746e9 100644
--- a/pod/perlfunc.pod
+++ b/pod/perlfunc.pod
@@ -1991,9 +1991,9 @@ and passed directly to C<execvp>, which is more efficient.  Examples:
 If you don't really want to execute the first argument, but want to lie
 to the program you are executing about its own name, you can specify
 the program you actually want to run as an "indirect object" (without a
-comma) in front of the LIST.  (This always forces interpretation of the
-LIST as a multivalued list, even if there is only a single scalar in
-the list.)  Example:
+comma) in front of the LIST, as in C<exec PROGRAM LIST>.  (This always
+forces interpretation of the LIST as a multivalued list, even if there
+is only a single scalar in the list.)  Example:
 
     $shell = '/bin/csh';
     exec $shell '-sh';    # pretend it's a login shell
@@ -2023,6 +2023,10 @@ program, passing it C<"surprise"> an argument.  The second version didn't;
 it tried to run a program named I<"echo surprise">, didn't find it, and set
 C<$?> to a non-zero value indicating failure.
 
+On Windows, only the C<exec PROGRAM LIST> indirect object syntax will
+reliably avoid using the shell; C<exec LIST>, even with more than one
+element, will fall back to the shell if the first spawn fails.
+
 Perl attempts to flush all files opened for output before the exec,
 but this may not be supported on some platforms (see L<perlport>).
 To be safe, you may need to set C<$|> ($AUTOFLUSH in English) or
@@ -7983,7 +7987,9 @@ entire argument is passed to the system's command shell for parsing
 (this is C</bin/sh -c> on Unix platforms, but varies on other
 platforms).  If there are no shell metacharacters in the argument,
 it is split into words and passed directly to C<execvp>, which is
-more efficient.
+more efficient.  On Windows, only the C<system PROGRAM LIST> syntax will
+reliably avoid using the shell; C<system LIST>, even with more than one
+element, will fall back to the shell if the first spawn fails.
 
 Perl will attempt to flush all files opened for
 output before any operation that may do a fork, but this may not be
diff --git a/pod/perlport.pod b/pod/perlport.pod
index 8f01e20..8b71a6e 100644
--- a/pod/perlport.pod
+++ b/pod/perlport.pod
@@ -1606,6 +1606,9 @@ Invokes VMS debugger. (VMS)
 
 =item exec
 
+C<exec LIST> without the use of indirect object syntax (C<exec PROGRAM LIST>)
+may fall back to trying the shell if the first spawn() fails.  (Win32)
+
 Does not automatically flush output handles on some platforms.
 (SunOS, Solaris, HP-UX)
 
@@ -2011,6 +2014,9 @@ the child program uses a compatible version of the emulation library.
 I<scalar> will call the native command line direct and no such emulation
 of a child Unix program will exists.  Mileage B<will> vary.  (S<RISC OS>)
 
+C<system LIST> without the use of indirect object syntax (C<system PROGRAM LIST>)
+may fall back to trying the shell if the first spawn() fails.  (Win32)
+
 Does not automatically flush output handles on some platforms.
 (SunOS, Solaris, HP-UX)
 
-- 
1.9.3

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jun 5, 2014

From @ikegami

On Thu, Jun 5, 2014 at 1​:26 PM, Thomas Sibley <perlbug-followup@​perl.org>
wrote​:

# New Ticket Created by Thomas Sibley
# Please include the string​: [perl #122046]
# in the subject line of all future correspondence about this issue.
# <URL​: https://rt-archive.perl.org/perl5/Ticket/Display.html?id=122046 >

Patch against blead attached.

The patch is accurate.

perl -e"system 'dir' and die;"
Volume in drive C is OS
...

perl -e"system { 'dir' } 'dir' and die;"
Died at -e line 1.

Keep in mind that dir is a shell builtin. Expected behaviour​:

$ perl -e'system "set" and die;'
Died at -e line 1.

$ perl -e'system { "set" } "set" and die;'
Died at -e line 1.

- Eric

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jun 5, 2014

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

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jun 5, 2014

From @bulk88

On Thu Jun 05 10​:26​:09 2014, tsibley wrote​:

Patch against blead attached.

Can we not have a huge block of code in a commit message? That should be in the ticket, not the git repo.

--
bulk88 ~ bulk88 at hotmail.com

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jun 5, 2014

From tsibley@cpan.org

On 06/05/2014 12​:57 PM, bulk88 via RT wrote​:

On Thu Jun 05 10​:26​:09 2014, tsibley wrote​:

Patch against blead attached.

Can we not have a huge block of code in a commit message? That should
be in the ticket, not the git repo.

I'm fine with a committer trimming it before applying, but why in the
world does it matter?

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jun 7, 2014

From @khwilliamson

On 06/05/2014 03​:28 PM, Thomas Sibley wrote​:

On 06/05/2014 12​:57 PM, bulk88 via RT wrote​:

On Thu Jun 05 10​:26​:09 2014, tsibley wrote​:

Patch against blead attached.

Can we not have a huge block of code in a commit message? That should
be in the ticket, not the git repo.

I'm fine with a committer trimming it before applying, but why in the
world does it matter?

It matters because some people do 'git log' all the time (me for
example), many times a day.

The main audience for a commit message are people who are looking to
keep up with what's changing in the Perl core. Some do it for idle
curiosity; some do it to see how changes might affect the portions of
Perl that they especially care about; and some are watchdogs trying to
prevent flaws from entering the code.

As such, the commit message should state briefly what is changing, and
if not obvious, why. Over time the commit messages have gotten more
detailed. I know that I am trying to write better commit messages than
I used to.

It becomes a pain to do excavating of commit messages as time goes by.
The blame log may have multiple changes to a line that one has to go
back through to find the original message that might have made the
purpose clear for a given line. For that reason, I think that much of
the detail for commits should be in comments which stay attached to
their code. And I do think that we tend to err in putting stuff in the
commit message that should be in the code.

Commit messages may well contain small snippets of code, or larger
snippets of pseudo-code (with unnecessary detail elided out) that
clarify what's going on. But try doing a 'git log' and see how ungainly
and distracting large snippets of code are in them

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jun 8, 2014

From @bulk88

On Sat Jun 07 16​:40​:37 2014, public@​khwilliamson.com wrote​:

On 06/05/2014 03​:28 PM, Thomas Sibley wrote​:

On 06/05/2014 12​:57 PM, bulk88 via RT wrote​:

On Thu Jun 05 10​:26​:09 2014, tsibley wrote​:

Patch against blead attached.

Can we not have a huge block of code in a commit message? That should
be in the ticket, not the git repo.

I'm fine with a committer trimming it before applying, but why in the
world does it matter?

It matters because some people do 'git log' all the time (me for
example), many times a day.

I worry more about repo size/CPU complexity/blame speed (I often use gitweb than my local blame tool since its faster in rendering time). I dont want to see a nytprof report page in the commit message.

--
bulk88 ~ bulk88 at hotmail.com

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jun 8, 2014

From tsibley@cpan.org

On 06/07/2014 04​:39 PM, Karl Williamson wrote​:

On 06/05/2014 03​:28 PM, Thomas Sibley wrote​:

On 06/05/2014 12​:57 PM, bulk88 via RT wrote​:

Can we not have a huge block of code in a commit message? That should
be in the ticket, not the git repo.

I'm fine with a committer trimming it before applying, but why in the
world does it matter?

It matters because some people do 'git log' all the time (me for
example), many times a day.

I can see how reading it in plain old `git log` would be a chore. I
find it easier to stay on top of commits by reading the first line
summaries and only reaching for the full message or even full diff when
I want to know more (using tools that make this easy). Given that, I
tend to not care as much how much detail is shoved into a commit
message, because it's only there if I want to see it and isn't in the
way otherwise.

Though I thought the code snippet explained the strange situation fairly
well, in retrospect now it's a poor excuse for not just describing the
behaviour in another sentence or two. A new patch is attached.

It becomes a pain to do excavating of commit messages as time goes by.
The blame log may have multiple changes to a line that one has to go
back through to find the original message that might have made the
purpose clear for a given line. For that reason, I think that much of
the detail for commits should be in comments which stay attached to
their code. And I do think that we tend to err in putting stuff in the
commit message that should be in the code.

This seems tangential to the commit message in question, unless you're
suggesting there should be comments added to the pod in this case?

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jun 8, 2014

From tsibley@cpan.org

0001-Document-that-exec-LIST-and-system-LIST-may-fall-bac.patch
From a00f27b28634d8bb8d007adf3bcf030da9f7cb7c Mon Sep 17 00:00:00 2001
From: Thomas Sibley <tsibley@cpan.org>
Date: Thu, 5 Jun 2014 10:17:42 -0700
Subject: [PATCH] Document that "exec LIST" and "system LIST" may fall back to
 the shell on Win32

As noted on p5p [1] and subsequently discussed [2].

The Win32 functions for handling exec() and system() attempt to
specially handle shell builtins by catching spawn failures and
re-attempting the spawn using the shell with the given argument LIST.

If "exec PROGRAM LIST" syntax (or the equivalent for system()) is used,
then only the specified PROGRAM will ever be run (although Perl will
still try the spawn twice on Win32 if PROGRAM doesn't exist or otherwise
can't be executed).

[1] http://www.nntp.perl.org/group/perl.perl5.porters/2014/04/msg214556.html
[2] http://www.nntp.perl.org/group/perl.perl5.porters/2014/04/msg214564.html
---
 pod/perlfunc.pod | 14 ++++++++++----
 pod/perlport.pod |  6 ++++++
 2 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/pod/perlfunc.pod b/pod/perlfunc.pod
index aff2cd5..ad746e9 100644
--- a/pod/perlfunc.pod
+++ b/pod/perlfunc.pod
@@ -1991,9 +1991,9 @@ and passed directly to C<execvp>, which is more efficient.  Examples:
 If you don't really want to execute the first argument, but want to lie
 to the program you are executing about its own name, you can specify
 the program you actually want to run as an "indirect object" (without a
-comma) in front of the LIST.  (This always forces interpretation of the
-LIST as a multivalued list, even if there is only a single scalar in
-the list.)  Example:
+comma) in front of the LIST, as in C<exec PROGRAM LIST>.  (This always
+forces interpretation of the LIST as a multivalued list, even if there
+is only a single scalar in the list.)  Example:
 
     $shell = '/bin/csh';
     exec $shell '-sh';    # pretend it's a login shell
@@ -2023,6 +2023,10 @@ program, passing it C<"surprise"> an argument.  The second version didn't;
 it tried to run a program named I<"echo surprise">, didn't find it, and set
 C<$?> to a non-zero value indicating failure.
 
+On Windows, only the C<exec PROGRAM LIST> indirect object syntax will
+reliably avoid using the shell; C<exec LIST>, even with more than one
+element, will fall back to the shell if the first spawn fails.
+
 Perl attempts to flush all files opened for output before the exec,
 but this may not be supported on some platforms (see L<perlport>).
 To be safe, you may need to set C<$|> ($AUTOFLUSH in English) or
@@ -7983,7 +7987,9 @@ entire argument is passed to the system's command shell for parsing
 (this is C</bin/sh -c> on Unix platforms, but varies on other
 platforms).  If there are no shell metacharacters in the argument,
 it is split into words and passed directly to C<execvp>, which is
-more efficient.
+more efficient.  On Windows, only the C<system PROGRAM LIST> syntax will
+reliably avoid using the shell; C<system LIST>, even with more than one
+element, will fall back to the shell if the first spawn fails.
 
 Perl will attempt to flush all files opened for
 output before any operation that may do a fork, but this may not be
diff --git a/pod/perlport.pod b/pod/perlport.pod
index 8f01e20..8b71a6e 100644
--- a/pod/perlport.pod
+++ b/pod/perlport.pod
@@ -1606,6 +1606,9 @@ Invokes VMS debugger. (VMS)
 
 =item exec
 
+C<exec LIST> without the use of indirect object syntax (C<exec PROGRAM LIST>)
+may fall back to trying the shell if the first spawn() fails.  (Win32)
+
 Does not automatically flush output handles on some platforms.
 (SunOS, Solaris, HP-UX)
 
@@ -2011,6 +2014,9 @@ the child program uses a compatible version of the emulation library.
 I<scalar> will call the native command line direct and no such emulation
 of a child Unix program will exists.  Mileage B<will> vary.  (S<RISC OS>)
 
+C<system LIST> without the use of indirect object syntax (C<system PROGRAM LIST>)
+may fall back to trying the shell if the first spawn() fails.  (Win32)
+
 Does not automatically flush output handles on some platforms.
 (SunOS, Solaris, HP-UX)
 
-- 
2.0.0

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jun 8, 2014

From @tonycoz

On Sat Jun 07 22​:16​:33 2014, tsibley wrote​:

Though I thought the code snippet explained the strange situation fairly
well, in retrospect now it's a poor excuse for not just describing the
behaviour in another sentence or two. A new patch is attached.

Thanks, applied as 94d4006.

Tony

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jun 8, 2014

@tonycoz - Status changed from 'open' to 'resolved'

@p5pRT p5pRT closed this Jun 8, 2014
@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jun 9, 2014

From @bulk88

On Thu Jun 05 10​:26​:09 2014, tsibley wrote​:

Patch against blead attached.

FWIW, at an offline event, I spoke with a Perl programmer whose day job had a very interesting security policy. His employer uses all Win 7 machines. Any attempt run the "cmd.exe" process (it won't actually start), will result in a very seriously investigated automatic alert to Information Security Department. The cmd.exe executable was replaced with a honeypot executable. This nearly resulted in him being terminated for attempting to hack his employer's IT system. He was using Strawberry Perl, and he got it whitelisted previously by the legal dept of his employer. Since there was no way to reliably get the cmd.exe attempt out of Strawberry Perl (or really Win32 Perl), and he is not allowed to compile his own Win32 Perl, he promptly switched Cygwin Perl and never looked back since cygperl will never run cmd.exe unless you put that string into system(). So there is atleast one person who would like a way to permanently disable the "shell" part of `` and system() on Win32 Perl. I don't think this should be implemented for 1 person that probably comfortably uses Cygperl, but if a chorus of requests come for this, (I'd guess it would be a special var), its something to implement.

--
bulk88 ~ bulk88 at hotmail.com

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jun 9, 2014

From @jandubois

On Sun, Jun 8, 2014 at 9​:10 PM, bulk88 via RT <perlbug-followup@​perl.org> wrote​:

FWIW, it doesn't make any sense to me, forbidding people from running
cmd.exe, but allowing to run perl.exe that in turn is allowed to
invoke any other executable on the system, but again not cmd.exe.

So they allow Cygwin to be installed, and /bin/sh is not a honeypot too?

I don't think Perl should be modified for this, but it is actually
easy to implement at the Perl script level​: Perl invokes cmd.exe via
PATH, so if you remove the system32 directory from $ENV{PATH}, then
system() will no longer be able to invoke it "by accident".

Cheers,
-Jan

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jun 9, 2014

From @jandubois

On Mon, Jun 9, 2014 at 10​:59 AM, Jan Dubois <jand@​activestate.com> wrote​:

[...] it is actually
easy to implement at the Perl script level​: Perl invokes cmd.exe via
PATH, so if you remove the system32 directory from $ENV{PATH}, then
system() will no longer be able to invoke it "by accident".

Tested sample code​:

$ENV{PATH} = join ';', grep !-f "$_/cmd.exe", split /;/, $ENV{PATH};
system("echo Hello World!");
warn $?>>8 if $?;

Cheers,
-Jan

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jun 9, 2014

From @bulk88

On Mon Jun 09 11​:00​:32 2014, jdb wrote​:

On Sun, Jun 8, 2014 at 9​:10 PM, bulk88 via RT <perlbug-
followup@​perl.org> wrote​:

FWIW, it doesn't make any sense to me, forbidding people from running
cmd.exe, but allowing to run perl.exe that in turn is allowed to
invoke any other executable on the system, but again not cmd.exe.

So they allow Cygwin to be installed, and /bin/sh is not a honeypot
too?

Yes, they approved Cygwin. cmd.exe wasn't banned by the employer but the ban came from an outside source the employer contracts with for security services and/or software, since the cmd.exe ban isn't an in-house policy, the person I spoke with would have to make the employer get rid of the contractor, which isn't going to happen. The employer previously didn't use Perl before hiring the person but they used everything else under the sun. /bin/sh isn't the "Windows Command Prompt" so its not on the 3rd party blacklist. I asked him if he could rename cmd.exe or bring it on a USB stick. He said he didn't know if it would catch him by CRC, and the next time he will "terminated with cause" if he is caught by the security system again, and he won't risk his job. I can't say who or what the employer is, for obvious reasons, but the owner isn't a natural person, or a single person, you can "talk with", and its run by a "board" or "council".

Now I won't speak about the person above's particular situation. For years I've seen articles of "hack your schools computer with the command prompt". Like http​://www.wikihow.com/Make-Command-Prompt-Appear-at-School http​://www.instructables.com/id/Getting-around-command-prompt-restrictions-for-rea/ , I've also read about the change the console's font, size and color to look like a word processor. So to see cmd.exe banned in a corporate security product, for libraries, schools, large corporations, isn't far fetched at all. But to use the "public library terminal" profile for a financial company or an engineering firm, lol

I don't think Perl should be modified for this, but it is actually
easy to implement at the Perl script level​: Perl invokes cmd.exe via
PATH, so if you remove the system32 directory from $ENV{PATH}, then
system() will no longer be able to invoke it "by accident".

I thought that would make loading XS modules or creating new processes impossible while path doesn't have system32. KnownDLLs does allow some core dlls to be found even outside of path, but the list is very small. I'll try suggesting that in the future if I ever hear of someone else with this problem again.

--
bulk88 ~ bulk88 at hotmail.com

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jun 10, 2014

From @ikegami

Looks like env var PERL5SHELL overrides the shell choice, so you could
point it to something other than cmd.exe. (See C<get_shell>)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant
You can’t perform that action at this time.