This is the mail archive of the
cygwin-patches
mailing list for the Cygwin project.
Re: [PATCH] Cygwin: exec: check execute bit prior to evaluating script
- From: Ken Brown <kbrown at cornell dot edu>
- To: "cygwin-patches at cygwin dot com" <cygwin-patches at cygwin dot com>
- Date: Tue, 6 Aug 2019 12:09:16 +0000
- Subject: Re: [PATCH] Cygwin: exec: check execute bit prior to evaluating script
- Arc-authentication-results: i=1; mx.microsoft.com 1;spf=pass smtp.mailfrom=cornell.edu;dmarc=pass action=none header.from=cornell.edu;dkim=pass header.d=cornell.edu;arc=none
- Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=nSQ4mpa2pBO+otZcUTU6lnUgM5FFd25dJgyKvCbtoLs=; b=aWAmDptwASFN4ATmAwAvMfcjov/ORWQMqyphuQ5N+mSHfCVxeLdRrAApxyKFQot1TgWkhejDtwHz4jppJGy3zmoIPIojWnBGIVylyCYG6vkYxrJslf2rNyAmbM5Tgag9omlr9cI/2KciehpXXSCavqriYnrifSfdWzH06fUEer8e00i9tD2bA3K2nLtKHmQlxuz92e/4ZtiTjOowfdjGj/YDcogBRVkhnjM8AbACc01KLv+kgIc/XqQ/jJkGrgq8OxC2PdVpZSqh3CwtXYsqZuogSrdqn+LhJ6pUnE54S4Z3LtWR68+caGaMFapj9DpCDtIlv1D/m9VUz9NdD17RKA==
- Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=UdabI+lPHE4NsBgGlOfcIOvJmVsPE6XLlJmfmYjrhmyrcz7c8XQwbQCCpID1HTaPNscvCu+mQj+r63ZnXqUSg7VqWf+gqm1dInrao+jSKwofYHzehDuivy5N9wSEsTlRNiUKcM5GykUFRMlCsp8+Z/Cn3LCWHBR30mXAHb42Ow+8Devs4M4TNQXPXvuhq7sekVg5UsntSSzKQRuzMFd2OD8HmARAYvMr4vH6flk7xlg6jSo5n8a0/W9aqnL21K1ptfM+GOQd4Boqmqvn5wrO6l1V0HVp6VhvgL8mMK8Fg1MGAdMBoEUMtedsQILDFz0DtiO5zJ8D8XDDDxbR1Rpatw==
- References: <20190806085354.14996-1-corinna@vinschen.de>
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