dash.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] Don't execute binary files if execve() returned ENOEXEC.
@ 2017-02-07  8:33 Adam Borowski
  2017-02-07 22:52 ` Jilles Tjoelker
  0 siblings, 1 reply; 4+ messages in thread
From: Adam Borowski @ 2017-02-07  8:33 UTC (permalink / raw)
  To: dash; +Cc: Adam Borowski

Both "dash -c foo" and "./foo" are supposed to be able to run hashbang-less
scripts, but attempts to execute common binary files tend to be nasty:
especially both ELF and PE tend to make dash create a bunch of files with
unprintable names, that in turn confuse some tools up to causing data loss.

Thus, let's read the first line and see if it looks like text.  This is a
variant of the approach used by bash and zsh; mksh instead checks for
signatures of a bunch of common file types.

POSIX says: "If the executable file is not a text file, the shell may bypass
this command execution.".

Signed-off-by: Adam Borowski <kilobyte@angband.pl>
---
This has been applied in Debian.  While technically it's only a "may" issue
in dash itself, and is triggered by user error (trying to exec files you
shouldn't exec), the fallout is nasty enough that the bug was classified as
serious.

The usual failure mode is to create files with names such as:
(per submitter, a PE):
90 d4 f6
(an ELF):
01 b0 40 40 08 01 40 38 02 40 04 03 01 05 40 40 ed ed
01 b0 40 40 f8 40 38 02 40 04 03 01 05 40 40 da da

diff --git a/src/exec.c b/src/exec.c
index ec0eadd..19ae752 100644
--- a/src/exec.c
+++ b/src/exec.c
@@ -148,6 +148,36 @@ shellexec(char **argv, const char *path, int idx)
 }
 
 
+/*
+ * Check if an executable that just failed with ENOEXEC shouldn't be
+ * considered a script (wrong-arch ELF/PE, junk accidentally set +x, etc).
+ * We check only the first line to allow binaries encapsulated in a shell
+ * script without proper quoting.  The first line, if not a hashbang, is
+ * likely to contain comments; even ancient encodings, at least popular
+ * ones, don't use 0x7f nor values below 0x1f other than whitespace (\t,
+ * \n, \v, \f, \r), ISO/IEC 2022 can have SI, SO and \e.
+ */
+STATIC int file_is_binary(const char *cmd)
+{
+	char buf[128];
+	int fd = open(cmd, O_RDONLY|O_NOCTTY);
+	if (fd == -1)
+		return 1;
+	int len = read(fd, buf, sizeof(buf));
+	close(fd);
+	for (int i = 0; i < len; ++i) {
+		char c = buf[i];
+		if (c >= 0 && c <= 8 ||
+		    c >= 16 && c <= 31 && c != 27 ||
+		    c == 0x7f)
+			return 1;
+		if (c == '\n')
+			return 0;
+	}
+	return 0;
+}
+
+
 STATIC void
 tryexec(char *cmd, char **argv, char **envp)
 {
@@ -162,6 +192,8 @@ repeat:
 	execve(cmd, argv, envp);
 #endif
 	if (cmd != path_bshell && errno == ENOEXEC) {
+		if (file_is_binary(cmd))
+			return;
 		*argv-- = cmd;
 		*argv = cmd = path_bshell;
 		goto repeat;
-- 
2.11.0


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2017-02-08 22:11 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-07  8:33 [PATCH] Don't execute binary files if execve() returned ENOEXEC Adam Borowski
2017-02-07 22:52 ` Jilles Tjoelker
2017-02-08  8:02   ` Adam Borowski
2017-02-08 22:11     ` Jilles Tjoelker

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).