From 8c35d65c43216bb840326ac7476a180e2ae36fe9 Mon Sep 17 00:00:00 2001 From: Denis Vlasenko Date: Fri, 27 Oct 2006 23:42:25 +0000 Subject: recursive_action: add depth param chmod: match coreutils versus following links --- coreutils/chmod.c | 98 ++++++++++++++++++++++++++++++++++++++++++------------- coreutils/chown.c | 8 +++-- coreutils/diff.c | 6 ++-- 3 files changed, 84 insertions(+), 28 deletions(-) (limited to 'coreutils') diff --git a/coreutils/chmod.c b/coreutils/chmod.c index c4f8fa0..b601504 100644 --- a/coreutils/chmod.c +++ b/coreutils/chmod.c @@ -20,9 +20,9 @@ #define OPT_VERBOSE (USE_DESKTOP(option_mask32 & 2) SKIP_DESKTOP(0)) #define OPT_CHANGED (USE_DESKTOP(option_mask32 & 4) SKIP_DESKTOP(0)) #define OPT_QUIET (USE_DESKTOP(option_mask32 & 8) SKIP_DESKTOP(0)) -#define OPT_STR ("-R" USE_DESKTOP("vcf")) +#define OPT_STR "R" USE_DESKTOP("vcf") -/* TODO: +/* coreutils: * chmod never changes the permissions of symbolic links; the chmod * system call cannot change their permissions. This is not a problem * since the permissions of symbolic links are never used. @@ -31,19 +31,26 @@ * symbolic links encountered during recursive directory traversals. */ -static int fileAction(const char *fileName, struct stat *statbuf, void* junk) +static int fileAction(const char *fileName, struct stat *statbuf, void* junk, int depth) { - mode_t newmode = statbuf->st_mode; + mode_t newmode; - // TODO: match GNU behavior: - // if (depth > 0 && S_ISLNK(statbuf->st_mode)) return TRUE; - // if (depth == 0) follow link + /* match coreutils behavior */ + if (depth == 0) { + /* statbuf holds lstat result, but we need stat (follow link) */ + if (stat(fileName, statbuf)) + goto err; + } else { /* depth > 0: skip links */ + if (S_ISLNK(statbuf->st_mode)) + return TRUE; + } + newmode = statbuf->st_mode; if (!bb_parse_mode((char *)junk, &newmode)) bb_error_msg_and_die("invalid mode: %s", (char *)junk); - if (chmod(fileName, statbuf->st_mode) == 0) { - if (OPT_VERBOSE /* -v verbose? or -c changed? */ + if (chmod(fileName, newmode) == 0) { + if (OPT_VERBOSE || (OPT_CHANGED && statbuf->st_mode != newmode) ) { printf("mode of '%s' changed to %04o (%s)\n", fileName, @@ -51,7 +58,8 @@ static int fileAction(const char *fileName, struct stat *statbuf, void* junk) } return TRUE; } - if (!OPT_QUIET) /* not silent (-f)? */ + err: + if (!OPT_QUIET) bb_perror_msg("%s", fileName); return FALSE; } @@ -62,30 +70,33 @@ int chmod_main(int argc, char **argv) char *arg, **argp; char *smode; - /* Convert first encountered -r into a-r, -w into a-w etc */ - argp = argv + 1; - while ((arg = *argp)) { + /* Convert first encountered -r into ar, -w into aw etc + * so that getopt would not eat it */ + argp = argv; + while ((arg = *++argp)) { /* Mode spec must be the first arg (sans -R etc) */ /* (protect against mishandling e.g. "chmod 644 -r") */ - if (arg[0] != '-') + if (arg[0] != '-') { + arg = NULL; break; + } /* An option. Not a -- or valid option? */ - if (arg[1] && !strchr(OPT_STR, arg[1])) { - argp[0] = xasprintf("a%s", arg); + if (arg[1] && !strchr("-"OPT_STR, arg[1])) { + arg[0] = 'a'; break; } - argp++; } - /* "chmod -rzzz abc" will say "invalid mode: a-rzzz"! - * It is easily fixable, but deemed not worth the code */ + /* Paerse options */ opt_complementary = "-2"; - getopt32(argc, argv, OPT_STR + 1); /* Reuse string */ + getopt32(argc, argv, ("-"OPT_STR) + 1); /* Reuse string */ argv += optind; - smode = *argv++; + /* Restore option-like mode if needed */ + if (arg) arg[0] = '-'; /* Ok, ready to do the deed now */ + smode = *argv++; do { if (!recursive_action(*argv, OPT_RECURSE, // recurse @@ -93,7 +104,8 @@ int chmod_main(int argc, char **argv) FALSE, // depth first fileAction, // file action fileAction, // dir action - smode) // user data + smode, // user data + 0) // depth ) { retval = EXIT_FAILURE; } @@ -101,3 +113,45 @@ int chmod_main(int argc, char **argv) return retval; } + +/* +Security: chmod is too important and too subtle. +This is a test script (busybox chmod versus coreutils). +Run it in empty dir. Probably requires bash. + +#!/bin/sh +function create() { + rm -rf $1; mkdir $1 + ( + cd $1 || exit 1 + mkdir dir + >up + >file + >dir/file + ln -s dir linkdir + ln -s file linkfile + ln -s ../up dir/up + ) +} +function test() { + (cd test1; $t1 $1) + (cd test2; $t2 $1) + (cd test1; ls -lR) >out1 + (cd test2; ls -lR) >out2 + echo "chmod $1" >out.diff + if ! diff -u out1 out2 >>out.diff; then exit 1; fi + mv out.diff out1.diff +} +t1="/tmp/busybox chmod" +t2="/usr/bin/chmod" +create test1; create test2 +test "a+w file" +test "a-w dir" +test "a+w linkfile" +test "a-w linkdir" +test "-R a+w file" +test "-R a-w dir" +test "-R a+w linkfile" +test "-R a-w linkdir" +test "a-r,a+x linkfile" +*/ diff --git a/coreutils/chown.c b/coreutils/chown.c index bef89ce..fddce7c 100644 --- a/coreutils/chown.c +++ b/coreutils/chown.c @@ -32,7 +32,7 @@ static int (*chown_func)(const char *, uid_t, gid_t) = chown; */ static int fileAction(const char *fileName, struct stat *statbuf, - void ATTRIBUTE_UNUSED *junk) + void ATTRIBUTE_UNUSED *junk, int depth) { // TODO: -H/-L/-P // if (depth ... && S_ISLNK(statbuf->st_mode)) .... @@ -75,7 +75,8 @@ int chown_main(int argc, char **argv) *groupName++ = '\0'; gid = get_ug_id(groupName, bb_xgetgrnam); } - if (--groupName != *argv) uid = get_ug_id(*argv, bb_xgetpwnam); + if (--groupName != *argv) + uid = get_ug_id(*argv, bb_xgetpwnam); ++argv; /* Ok, ready to do the deed now */ @@ -86,7 +87,8 @@ int chown_main(int argc, char **argv) FALSE, // depth first fileAction, // file action fileAction, // dir action - NULL) // user data + NULL, // user data + 0) // depth ) { retval = EXIT_FAILURE; } diff --git a/coreutils/diff.c b/coreutils/diff.c index 2915d40..f26bcca 100644 --- a/coreutils/diff.c +++ b/coreutils/diff.c @@ -1030,7 +1030,7 @@ static int dir_strcmp(const void *p1, const void *p2) /* This function adds a filename to dl, the directory listing. */ static int add_to_dirlist(const char *filename, - struct stat ATTRIBUTE_UNUSED * sb, void *userdata) + struct stat ATTRIBUTE_UNUSED * sb, void *userdata, int depth) { dl_count++; dl = xrealloc(dl, dl_count * sizeof(char *)); @@ -1067,7 +1067,7 @@ static char **get_dir(char *path) /* Now fill dl with a listing. */ if (cmd_flags & FLAG_r) recursive_action(path, TRUE, TRUE, FALSE, add_to_dirlist, NULL, - userdata); + userdata, 0); else { DIR *dp; struct dirent *ep; @@ -1076,7 +1076,7 @@ static char **get_dir(char *path) while ((ep = readdir(dp))) { if ((!strcmp(ep->d_name, "..")) || (!strcmp(ep->d_name, "."))) continue; - add_to_dirlist(ep->d_name, NULL, NULL); + add_to_dirlist(ep->d_name, NULL, NULL, 0); } closedir(dp); } -- cgit v1.1