All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jilles Tjoelker <jilles@stack.nl>
To: Adam Borowski <kilobyte@angband.pl>
Cc: dash@vger.kernel.org
Subject: Re: [PATCH] Don't execute binary files if execve() returned ENOEXEC.
Date: Tue, 7 Feb 2017 23:52:08 +0100	[thread overview]
Message-ID: <20170207225208.GA743@stack.nl> (raw)
In-Reply-To: <20170207083307.14881-1-kilobyte@angband.pl>

On Tue, Feb 07, 2017 at 09:33:07AM +0100, Adam Borowski wrote:
> 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

In FreeBSD sh, I have done this slightly differently (since 2011), based
on POSIX's definition of a text file in XBD 3:

] A file that contains characters organized into zero or more lines. The
] lines do not contain NUL characters and none can exceed {LINE_MAX} bytes
] in length, including the <newline> character.

The check is simply for a 0 byte in the first 256 bytes (how many bytes
are read by pread() for 256 bytes). A file containing the byte 8, for
example, can still be a text file per POSIX's definition.

This check might cause a terse script with binary to fail to execute,
but I have not received bug reports about that.

Stopping the check with a \n will cause a PNG header to be considered
text.

Also, FreeBSD sh uses O_NONBLOCK when opening the file and pread() to
read the data, in order to minimize the potential for modifying things
by reading.

-- 
Jilles Tjoelker

  reply	other threads:[~2017-02-07 22:52 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-02-07  8:33 [PATCH] Don't execute binary files if execve() returned ENOEXEC Adam Borowski
2017-02-07 22:52 ` Jilles Tjoelker [this message]
2017-02-08  8:02   ` Adam Borowski
2017-02-08 22:11     ` Jilles Tjoelker

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=20170207225208.GA743@stack.nl \
    --to=jilles@stack.nl \
    --cc=dash@vger.kernel.org \
    --cc=kilobyte@angband.pl \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.