Frans Klaver <fransklaver@xxxxxxxxx> writes:
> +#ifndef WIN32
> +static int is_in_group(gid_t gid)
> ...
> +static int have_read_execute_permissions(const char *path)
> +{
> + struct stat s;
> + trace_printf("checking '%s'\n", path);
> +
> + if (stat(path, &s) < 0) {
> + ...
> + /* check world permissions */
> + if ((s.st_mode&(S_IXOTH|S_IROTH)) == (S_IXOTH|S_IROTH))
> + return 1;
Hmm, do you need to do this with stat(2)?
Wouldn't access(2) with R_OK|X_OK give you exactly what you want without
this much trouble?
I also think that your permission check is incorrectly implemented.
$ cd /var/tmp && date >j && chmod 044 j && ls -l j
----r--r-- 1 junio junio 29 Dec 6 14:32 j
$ cat j
cat: j: Permission denied
$ su pogo
Password:
$ cat j
Tue Dec 6 14:32:23 PST 2011
That's a world-readable but unreadable-only-to-me file.
> +static void diagnose_execvp_eacces(const char *cmd, const char **argv)
> +{
> + /* man 2 execve states that EACCES is returned for:
/*
* Just a style, but we tend to write multi-line comment like
* this, without anything else on opening and closing lines of
* the comment block.
*/
> + * - The file system is mounted noexec
> + */
> + struct strbuf sb = STRBUF_INIT;
> + char *path = getenv("PATH");
> + char *next;
> +
> + if (strchr(cmd, '/')) {
> + if (!have_read_execute_permissions(cmd))
> + error("no read/execute permissions on '%s'\n", cmd);
> + return;
> + }
Ok, execvp() failed and "cmd" has at least one slash, so we know we did
not look for it in $PATH. We check only one and return (did you need
getenv() in that case?).
> + for (;;) {
> + next = strchrnul(path, ':');
> + if (path < next)
> + strbuf_add(&sb, path, next - path);
> + else
> + strbuf_addch(&sb, '.');
Nice touch that you did not forget an empty component on $PATH.
> + if (!have_read_execute_permissions(sb.buf))
> + error("no read/execute permissions on '%s'\n", sb.buf);
Don't you want to continue here upon error, after resetting sb? You just
saw the directory is unreadble, so you know next file_exists() will fail
before you try it.
> + if (sb.len && sb.buf[sb.len - 1] != '/')
> + strbuf_addch(&sb, '/');
> + strbuf_addstr(&sb, cmd);
> +
> + if (file_exists(sb.buf)) {
> + if (!have_read_execute_permissions(sb.buf))
> + 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);
Does "warn()" do the right thing for multi-line strings like this?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html