Re: [PATCH] Cygwin: exec: check execute bit prior to evaluating script

[Corinna Vinschen <corinna-cygwin at cygwin dot com>]

On Aug  6 12:09, Ken Brown wrote:
> On 8/6/2019 4:53 AM, Corinna Vinschen wrote:
> > From: Corinna Vinschen <corinna-cygwin@cygwin.com>
> > 
> > When the exec family of functions is called for a script-like
> > file, the av::setup function handles the exec[vl]p case as
> > well.  The execve case for files not starting with a she-bang
> > is handled first by returning ENOEXEC.  Only after that, the
> > file's executability is checked.
> > 
> > This leads to the problem that ENOEXEC is returned for non-executable
> > files as well.  A calling shell interprets this as a file it should try
> > to run as script.  This is not desired for non-executable files.
> > 
> > Fix this problem by checking the file for executability first.  Only
> > after that, follow the other potential code paths.
> > ---
> >   winsup/cygwin/spawn.cc | 12 ++++++------
> >   1 file changed, 6 insertions(+), 6 deletions(-)
> > 
> > diff --git a/winsup/cygwin/spawn.cc b/winsup/cygwin/spawn.cc
> > index 7f7af4449da1..d95772802f8f 100644
> > --- a/winsup/cygwin/spawn.cc
> > +++ b/winsup/cygwin/spawn.cc
> > @@ -1172,6 +1172,12 @@ av::setup (const char *prog_arg, path_conv& real_path, const char *ext,
> >   	  }
> >   	UnmapViewOfFile (buf);
> >     just_shell:
> > +	/* Check if script is executable.  Otherwise we start non-executable
> > +	   scripts successfully, which is incorrect behaviour. */
> > +	if (real_path.has_acls ()
> > +	    && check_file_access (real_path, X_OK, true) < 0)
> > +	  return -1;	/* errno is already set. */
> > +
> >   	if (!pgm)
> >   	  {
> >   	    if (!p_type_exec)
> > @@ -1188,12 +1194,6 @@ av::setup (const char *prog_arg, path_conv& real_path, const char *ext,
> >   	    arg1 = NULL;
> >   	  }
> >   
> > -	/* Check if script is executable.  Otherwise we start non-executable
> > -	   scripts successfully, which is incorrect behaviour. */
> > -	if (real_path.has_acls ()
> > -	    && check_file_access (real_path, X_OK, true) < 0)
> > -	  return -1;	/* errno is already set. */
> > -
> >   	/* Replace argv[0] with the full path to the script if this is the
> >   	   first time through the loop. */
> >   	replace0_maybe (prog_arg);
> 
> LGTM, and I've confirmed that it fixes the problem reported in 
> http://www.cygwin.org/ml/cygwin/2019-08/msg00054.html.
> 
> Ken

Thanks!  Pushed.


Corinna

-- 
Corinna Vinschen
Cygwin Maintainer

Attachment: signature.asc
Description: PGP signature