All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jonathan Nieder <jrnieder@gmail.com>
To: dash@vger.kernel.org
Cc: Christoph Egger <christoph@debian.org>,
	Petr Salinger <Petr.Salinger@seznam.cz>,
	Herbert Xu <herbert@gondor.apana.org.au>
Subject: [PATCH] [BUILTIN] Make "test -x" sane again when run as root
Date: Mon, 26 Sep 2011 16:16:37 -0500	[thread overview]
Message-ID: <20110926211636.GA6645@elie> (raw)

When dash switched from its own emulation to the true faccessat in
v0.5.7~54 (2010-04-02), on some platforms (e.g., old versions of
glibc-bsd), "test -x <path>" started returning true on all files when
run as root.  This violates POSIX.1-2008 §4.4 "File Access
Permission", which says:

	If execute permission is requested, access shall be granted
	if execute permission is granted to at least one user by the
	file permission bits or by an alternate access control
	mechanism; otherwise, access shall be denied.

Unfortunately, for historical reasons, access() and faccessat() are
allowed by POSIX to return success for X_OK when the current process
is privileged even when the above condition is not fulfilled and
actual execution would fail.  Work around this by checking the
permissions bits when mode == X_OK and geteuid() == 0.

Reported-by: Christoph Egger <christoph@debian.org>
Analysis-by: Petr Salinger <Petr.Salinger@seznam.cz>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Thanks, all, and sorry to take so long in sending this patch out.

Thoughts?

 src/bltin/test.c |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/src/bltin/test.c b/src/bltin/test.c
index 90135e14..1093b59f 100644
--- a/src/bltin/test.c
+++ b/src/bltin/test.c
@@ -485,8 +485,19 @@ equalf (const char *f1, const char *f2)
 }
 
 #ifdef HAVE_FACCESSAT
+static int has_exec_bit_set(const char *path)
+{
+	struct stat64 st;
+
+	if (stat64(path, &st))
+		return 0;
+	return st.st_mode & (S_IXUSR | S_IXGRP | S_IXOTH);
+}
+
 static int test_file_access(const char *path, int mode)
 {
+	if (mode == X_OK && geteuid() == 0 && !has_exec_bit_set(path))
+		return 0;
 	return !faccessat(AT_FDCWD, path, mode, AT_EACCESS);
 }
 #else	/* HAVE_FACCESSAT */
-- 
1.7.7.rc1


             reply	other threads:[~2011-09-26 21:16 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-09-26 21:16 Jonathan Nieder [this message]
2011-09-26 22:16 ` [PATCH] [BUILTIN] Make "test -x" sane again when run as root Jonathan Nieder
2011-09-27 13:45   ` Herbert Xu
2011-09-27 23:19     ` [PATCH v2] [BUILTIN] Fix "test -x" as root on FreeBSD 8 Jonathan Nieder
2014-11-17 14:49       ` Herbert Xu

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=20110926211636.GA6645@elie \
    --to=jrnieder@gmail.com \
    --cc=Petr.Salinger@seznam.cz \
    --cc=christoph@debian.org \
    --cc=dash@vger.kernel.org \
    --cc=herbert@gondor.apana.org.au \
    /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.