diff options
Diffstat (limited to '')
| -rw-r--r-- | src/bltin/test.c | 203 |
1 files changed, 157 insertions, 46 deletions
diff --git a/src/bltin/test.c b/src/bltin/test.c index d1458df..b7188df 100644 --- a/src/bltin/test.c +++ b/src/bltin/test.c @@ -151,8 +151,7 @@ static int equalf(const char *, const char *); #ifdef HAVE_FACCESSAT static int test_file_access(const char *, int); #else -static int test_st_mode(const struct stat64 *, int); -static int bash_group_member(gid_t); +static int test_access(const struct stat64 *, int); #endif #ifdef HAVE_FACCESSAT @@ -397,11 +396,11 @@ filstat(char *nm, enum token mode) switch (mode) { #ifndef HAVE_FACCESSAT case FILRD: - return test_st_mode(&s, R_OK); + return test_access(&s, R_OK); case FILWR: - return test_st_mode(&s, W_OK); + return test_access(&s, W_OK); case FILEX: - return test_st_mode(&s, X_OK); + return test_access(&s, X_OK); #endif case FILEXIST: return 1; @@ -537,53 +536,165 @@ static int test_file_access(const char *path, int mode) } #else /* HAVE_FACCESSAT */ /* - * Similar to what access(2) does, but uses the effective uid and gid. - * Doesn't make the mistake of telling root that any file is executable. - * Returns non-zero if the file is accessible. + * The manual, and IEEE POSIX 1003.2, suggests this should check the mode bits, + * not use access(): + * + * True shall indicate only that the write flag is on. The file is not + * writable on a read-only file system even if this test indicates true. + * + * Unfortunately IEEE POSIX 1003.1-2001, as quoted in SuSv3, says only: + * + * True shall indicate that permission to read from file will be granted, + * as defined in "File Read, Write, and Creation". + * + * and that section says: + * + * When a file is to be read or written, the file shall be opened with an + * access mode corresponding to the operation to be performed. If file + * access permissions deny access, the requested operation shall fail. + * + * and of course access permissions are described as one might expect: + * + * * If a process has the appropriate privilege: + * + * * If read, write, or directory search permission is requested, + * access shall be granted. + * + * * 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. + * + * * Otherwise: + * + * * The file permission bits of a file contain read, write, and + * execute/search permissions for the file owner class, file group + * class, and file other class. + * + * * Access shall be granted if an alternate access control mechanism + * is not enabled and the requested access permission bit is set for + * the class (file owner class, file group class, or file other class) + * to which the process belongs, or if an alternate access control + * mechanism is enabled and it allows the requested access; otherwise, + * access shall be denied. + * + * and when I first read this I thought: surely we can't go about using + * open(O_WRONLY) to try this test! However the POSIX 1003.1-2001 Rationale + * section for test does in fact say: + * + * On historical BSD systems, test -w directory always returned false + * because test tried to open the directory for writing, which always + * fails. + * + * and indeed this is in fact true for Seventh Edition UNIX, UNIX 32V, and UNIX + * System III, and thus presumably also for BSD up to and including 4.3. + * + * Secondly I remembered why using open() and/or access() are bogus. They + * don't work right for detecting read and write permissions bits when called + * by root. + * + * Interestingly the 'test' in 4.4BSD was closer to correct (as per + * 1003.2-1992) and it was implemented efficiently with stat() instead of + * open(). + * + * This was apparently broken in NetBSD around about 1994/06/30 when the old + * 4.4BSD implementation was replaced with a (arguably much better coded) + * implementation derived from pdksh. + * + * Note that modern pdksh is yet different again, but still not correct, at + * least not w.r.t. 1003.2-1992. + * + * As I think more about it and read more of the related IEEE docs I don't like + * that wording about 'test -r' and 'test -w' in 1003.1-2001 at all. I very + * much prefer the original wording in 1003.2-1992. It is much more useful, + * and so that's what I've implemented. + * + * (Note that a strictly conforming implementation of 1003.1-2001 is in fact + * totally useless for the case in question since its 'test -w' and 'test -r' + * can never fail for root for any existing files, i.e. files for which 'test + * -e' succeeds.) + * + * The rationale for 1003.1-2001 suggests that the wording was "clarified" in + * 1003.1-2001 to align with the 1003.2b draft. 1003.2b Draft 12 (July 1999), + * which is the latest copy I have, does carry the same suggested wording as is + * in 1003.1-2001, with its rationale saying: + * + * This change is a clarification and is the result of interpretation + * request PASC 1003.2-92 #23 submitted for IEEE Std 1003.2-1992. + * + * That interpretation can be found here: + * + * http://www.pasc.org/interps/unofficial/db/p1003.2/pasc-1003.2-23.html + * + * Not terribly helpful, unfortunately. I wonder who that fence sitter was. + * + * Worse, IMVNSHO, I think the authors of 1003.2b-D12 have mis-interpreted the + * PASC interpretation and appear to be gone against at least one widely used + * implementation (namely 4.4BSD). The problem is that for file access by root + * this means that if test '-r' and '-w' are to behave as if open() were called + * then there's no way for a shell script running as root to check if a file + * has certain access bits set other than by the grotty means of interpreting + * the output of 'ls -l'. This was widely considered to be a bug in V7's + * "test" and is, I believe, one of the reasons why direct use of access() was + * avoided in some more recent implementations! + * + * I have always interpreted '-r' to match '-w' and '-x' as per the original + * wording in 1003.2-1992, not the other way around. I think 1003.2b goes much + * too far the wrong way without any valid rationale and that it's best if we + * stick with 1003.2-1992 and test the flags, and not mimic the behaviour of + * open() since we already know very well how it will work -- existance of the + * file is all that matters to open() for root. + * + * Unfortunately the SVID is no help at all (which is, I guess, partly why + * we're in this mess in the first place :-). + * + * The SysV implementation (at least in the 'test' builtin in /bin/sh) does use + * access(name, 2) even though it also goes to much greater lengths for '-x' + * matching the 1003.2-1992 definition (which is no doubt where that definition + * came from). + * + * The ksh93 implementation uses access() for '-r' and '-w' if + * (euid==uid&&egid==gid), but uses st_mode for '-x' iff running as root. + * i.e. it does strictly conform to 1003.1-2001 (and presumably 1003.2b). */ -static int -test_st_mode(const struct stat64 *st, int mode) +static int test_access(const struct stat64 *sp, int stmode) { - int euid = geteuid(); + gid_t *groups; + register int n; + uid_t euid; + int maxgroups; + /* + * I suppose we could use access() if not running as root and if we are + * running with ((euid == uid) && (egid == gid)), but we've already + * done the stat() so we might as well just test the permissions + * directly instead of asking the kernel to do it.... + */ + euid = geteuid(); if (euid == 0) { - /* Root can read or write any file. */ - if (mode != X_OK) + if (stmode != X_OK) return 1; - /* Root can execute any file that has any one of the execute - bits set. */ - mode = S_IXUSR | S_IXGRP | S_IXOTH; - } else if (st->st_uid == euid) - mode <<= 6; - else if (bash_group_member(st->st_gid)) - mode <<= 3; - - return st->st_mode & mode; -} - -/* Return non-zero if GID is one that we have in our groups list. */ -static int -bash_group_member(gid_t gid) -{ - register int i; - gid_t *group_array; - int ngroups; - - /* Short-circuit if possible, maybe saving a call to getgroups(). */ - if (gid == getgid() || gid == getegid()) - return (1); - - ngroups = getgroups(0, NULL); - group_array = stalloc(ngroups * sizeof(gid_t)); - if ((getgroups(ngroups, group_array)) != ngroups) - return (0); - - /* Search through the list looking for GID. */ - for (i = 0; i < ngroups; i++) - if (gid == group_array[i]) - return (1); + /* any bit is good enough */ + stmode = (stmode << 6) | (stmode << 3) | stmode; + } else if (sp->st_uid == euid) + stmode <<= 6; + else if (sp->st_gid == getegid()) + stmode <<= 3; + else { + /* XXX stolen almost verbatim from ksh93.... */ + /* on some systems you can be in several groups */ + maxgroups = getgroups(0, NULL); + groups = stalloc(maxgroups * sizeof(*groups)); + n = getgroups(maxgroups, groups); + while (--n >= 0) { + if (groups[n] == sp->st_gid) { + stmode <<= 3; + break; + } + } + } - return (0); + return sp->st_mode & stmode; } #endif /* HAVE_FACCESSAT */ |