git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Frans Klaver <fransklaver@gmail.com>
To: git@vger.kernel.org
Cc: Junio C Hamano <gitster@pobox.com>, Frans Klaver <fransklaver@gmail.com>
Subject: [PATCH 2/2] run-command: Add interpreter permissions check
Date: Tue,  6 Dec 2011 22:38:23 +0100	[thread overview]
Message-ID: <1323207503-26581-3-git-send-email-fransklaver@gmail.com> (raw)
In-Reply-To: <1323207503-26581-1-git-send-email-fransklaver@gmail.com>

If a script is started and the interpreter of that script given in the
shebang cannot be started due to permissions, we can get a rather
obscure situation. All permission checks pass for the script itself,
but we still get EACCES from execvp.

Try to find out if the above is the case and warn the user about it.

Signed-off-by: Frans Klaver <fransklaver@gmail.com>
---
 run-command.c          |   66 +++++++++++++++++++++++++++++++++++++++++++----
 t/t0061-run-command.sh |   22 ++++++++++++++++
 2 files changed, 82 insertions(+), 6 deletions(-)

diff --git a/run-command.c b/run-command.c
index 5e38c5a..b8cf8d4 100644
--- a/run-command.c
+++ b/run-command.c
@@ -194,6 +194,63 @@ static int have_read_execute_permissions(const char *path)
 	return 0;
 }
 
+static void check_interpreter(const char *cmd)
+{
+	FILE *f;
+	struct strbuf sb = STRBUF_INIT;
+	/* bash reads an 80 character line when determining the interpreter.
+	 * BSD apparently only allows 32 characters, as it is the size of
+	 * your average binary executable header.
+	 */
+	char firstline[80];
+	char *interpreter = NULL;
+	size_t s, i;
+
+	f = fopen(cmd, "r");
+	if (!f) {
+		error("cannot open file '%s': %s\n", cmd, strerror(errno));
+		return;
+	}
+
+	s = fread(firstline, 1, sizeof(firstline), f);
+	if (s < 2) {
+		trace_printf("cannot determine file type");
+		fclose(f);
+		return;
+	}
+
+	if (firstline[0] != '#' || firstline[1] != '!') {
+		trace_printf("file '%s' is not a script or"
+				" is a script without '#!'", cmd);
+		fclose(f);
+		return;
+	}
+
+	/* see if the given path has the executable bit set */
+	for (i = 2; i < s; i++) {
+		if (!interpreter && firstline[i] != ' ' && firstline[i] != '\t')
+			interpreter = firstline + i;
+
+		if (interpreter && (firstline[i] == ' ' ||
+				firstline[i] == '\n')) {
+			strbuf_add(&sb, interpreter,
+					(firstline + i) - interpreter);
+			break;
+		}
+	}
+	if (!sb.len) {
+		error("could not determine interpreter");
+		strbuf_release(&sb);
+		return;
+	}
+
+	if (!have_read_execute_permissions(sb.buf))
+		error("bad interpreter: no read/execute permissions on '%s'\n",
+				sb.buf);
+
+	strbuf_release(&sb);
+}
+
 static void diagnose_execvp_eacces(const char *cmd, const char **argv)
 {
 	/* man 2 execve states that EACCES is returned for:
@@ -209,8 +266,8 @@ static void diagnose_execvp_eacces(const char *cmd, const char **argv)
 	char *next;
 
 	if (strchr(cmd, '/')) {
-		if (!have_read_execute_permissions(cmd))
-			error("no read/execute permissions on '%s'\n", cmd);
+		if (have_read_execute_permissions(cmd))
+			check_interpreter(cmd);
 		return;
 	}
 
@@ -233,10 +290,7 @@ static void diagnose_execvp_eacces(const char *cmd, const char **argv)
 				error("no read/execute permissions on '%s'\n",
 						sb.buf);
 			else
-				warn("file '%s' exists and permissions "
-				"seem OK.\nIf this is a script, see if you "
-				"have sufficient privileges to run the "
-				"interpreter", sb.buf);
+				check_interpreter(sb.buf);
 		}
 
 		strbuf_release(&sb);
diff --git a/t/t0061-run-command.sh b/t/t0061-run-command.sh
index b39bd16..39bfaef 100755
--- a/t/t0061-run-command.sh
+++ b/t/t0061-run-command.sh
@@ -13,6 +13,18 @@ cat >hello-script <<-EOF
 EOF
 >empty
 
+cat >someinterpreter <<-EOF
+	#!$SHELL_PATH
+	cat hello-script
+EOF
+>empty
+
+cat >incorrect-interpreter-script <<-EOF
+	#!someinterpreter
+	cat hello-script
+EOF
+>empty
+
 test_expect_success 'start_command reports ENOENT' '
 	test-run-command start-command-ENOENT ./does-not-exist
 '
@@ -48,4 +60,14 @@ test_expect_success POSIXPERM 'run_command reports EACCES, search path permision
 	grep "no read/execute permissions on" err
 '
 
+test_expect_success POSIXPERM 'run_command reports EACCES, interpreter fails' '
+	cat incorrect-interpreter-script >hello.sh &&
+	chmod +x hello.sh &&
+	chmod -x someinterpreter &&
+	test_must_fail test-run-command run-command ./hello.sh 2>err &&
+
+	grep "fatal: cannot exec.*hello.sh" err &&
+	grep "bad interpreter" err
+'
+
 test_done
-- 
1.7.8

  parent reply	other threads:[~2011-12-06 21:39 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-11-21 21:53 [PATCH] run-command.c: Accept EACCES as command not found Frans Klaver
2011-11-21 22:13 ` Junio C Hamano
2011-11-21 23:06   ` Frans Klaver
2011-11-21 23:54     ` Junio C Hamano
2011-11-22  9:31       ` Frans Klaver
2011-11-23  8:17         ` Frans Klaver
2011-11-23 12:04           ` Nguyen Thai Ngoc Duy
2011-11-23 13:25             ` Frans Klaver
2011-11-23 22:55           ` Frans Klaver
2011-12-06 21:38             ` [PATCH 0/2] run-command: Add EACCES diagnostics Frans Klaver
2011-12-06 21:38               ` [PATCH 1/2] run-command: Add checks after execvp fails with EACCES Frans Klaver
2011-12-06 22:35                 ` Junio C Hamano
2011-12-07  8:31                   ` Frans Klaver
2011-12-08 21:44                   ` Frans Klaver
2011-12-09 17:23                     ` Junio C Hamano
2011-12-09 21:35                       ` Frans Klaver
2011-12-06 21:38               ` Frans Klaver [this message]
2011-12-06 22:47                 ` [PATCH 2/2] run-command: Add interpreter permissions check Junio C Hamano
2011-12-07  8:37                   ` Frans Klaver
2011-12-13 15:08             ` [PATCH 0/2 v2] run-command: Add eacces diagnostics Frans Klaver
2011-12-13 15:08               ` [PATCH 1/2] run-command: Add checks after execvp fails with EACCES Frans Klaver
2011-12-13 19:01                 ` Junio C Hamano
2011-12-14 14:31                   ` Frans Klaver
2011-12-14 22:06                     ` Frans Klaver
2011-12-13 15:08               ` [PATCH 2/2] run-command: Add interpreter permissions check Frans Klaver

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1323207503-26581-3-git-send-email-fransklaver@gmail.com \
    --to=fransklaver@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).