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 --- archival/tar.c | 4 +- coreutils/chmod.c | 98 +++++++++++++++++++++++++++++++++++++----------- coreutils/chown.c | 8 ++-- coreutils/diff.c | 6 +-- findutils/find.c | 6 +-- findutils/grep.c | 5 ++- include/libbb.h | 8 ++-- libbb/recursive_action.c | 40 ++++++++++++-------- modutils/insmod.c | 6 +-- 9 files changed, 123 insertions(+), 58 deletions(-) diff --git a/archival/tar.c b/archival/tar.c index d96d202..db812e0 100644 --- a/archival/tar.c +++ b/archival/tar.c @@ -301,7 +301,7 @@ static int exclude_file(const llist_t *excluded_files, const char *file) # endif static int writeFileToTarball(const char *fileName, struct stat *statbuf, - void *userData) + void *userData, int depth) { struct TarBallInfo *tbInfo = (struct TarBallInfo *) userData; const char *header_name; @@ -473,7 +473,7 @@ static int writeTarFile(const int tar_fd, const int verboseFlag, /* Read the directory/files and iterate over them one at a time */ while (include) { if (!recursive_action(include->data, TRUE, dereferenceFlag, - FALSE, writeFileToTarball, writeFileToTarball, &tbInfo)) + FALSE, writeFileToTarball, writeFileToTarball, &tbInfo, 0)) { errorFlag = TRUE; } 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); } diff --git a/findutils/find.c b/findutils/find.c index 62cb5d7..8618f3a 100644 --- a/findutils/find.c +++ b/findutils/find.c @@ -56,7 +56,7 @@ static int num_matches; static int exec_opt; #endif -static int fileAction(const char *fileName, struct stat *statbuf, void* junk) +static int fileAction(const char *fileName, struct stat *statbuf, void* junk, int depth) { #ifdef CONFIG_FEATURE_FIND_XDEV if (S_ISDIR(statbuf->st_mode) && xdev_count) { @@ -307,12 +307,12 @@ int find_main(int argc, char **argv) if (firstopt == 1) { if (!recursive_action(".", TRUE, dereference, FALSE, fileAction, - fileAction, NULL)) + fileAction, NULL, 0)) status = EXIT_FAILURE; } else { for (i = 1; i < firstopt; i++) { if (!recursive_action(argv[i], TRUE, dereference, FALSE, fileAction, - fileAction, NULL)) + fileAction, NULL, 0)) status = EXIT_FAILURE; } } diff --git a/findutils/grep.c b/findutils/grep.c index b76a17a..8bb38f9 100644 --- a/findutils/grep.c +++ b/findutils/grep.c @@ -290,7 +290,7 @@ static void load_regexes_from_file(llist_t *fopt) } -static int file_action_grep(const char *filename, struct stat *statbuf, void* matched) +static int file_action_grep(const char *filename, struct stat *statbuf, void* matched, int depth) { FILE *file = fopen(filename, "r"); if (file == NULL) { @@ -315,7 +315,8 @@ static int grep_dir(const char *dir) /* depthFirst= */ 1, /* fileAction= */ file_action_grep, /* dirAction= */ NULL, - /* userData= */ &matched); + /* userData= */ &matched, + /* depth= */ 0); return matched; } diff --git a/include/libbb.h b/include/libbb.h index 169964b..9f9a2b8 100644 --- a/include/libbb.h +++ b/include/libbb.h @@ -194,10 +194,10 @@ extern int is_directory(const char *name, int followLinks, struct stat *statBuf) extern int remove_file(const char *path, int flags); extern int copy_file(const char *source, const char *dest, int flags); extern int recursive_action(const char *fileName, int recurse, - int followLinks, int depthFirst, - int (*fileAction) (const char *fileName, struct stat* statbuf, void* userData), - int (*dirAction) (const char *fileName, struct stat* statbuf, void* userData), - void* userData); + int followLinks, int depthFirst, + int (*fileAction) (const char *fileName, struct stat* statbuf, void* userData, int depth), + int (*dirAction) (const char *fileName, struct stat* statbuf, void* userData, int depth), + void* userData, int depth); extern int device_open(const char *device, int mode); extern int get_console_fd(void); extern char *find_block_device(char *path); diff --git a/libbb/recursive_action.c b/libbb/recursive_action.c index 0d6567d..ddaf9b8 100644 --- a/libbb/recursive_action.c +++ b/libbb/recursive_action.c @@ -11,7 +11,6 @@ #undef DEBUG_RECURS_ACTION - /* * Walk down all the directories under the specified * location, and do something (something specified @@ -23,16 +22,23 @@ * is so stinking huge. */ -static int true_action(const char *fileName, struct stat *statbuf, void* userData) +static int true_action(const char *fileName, struct stat *statbuf, void* userData, int depth) { return TRUE; } +/* + * followLinks=0/1 differs mainly in handling of links to dirs. + * 0: lstat(statbuf). Calls fileAction on link name even if points to dir. + * 1: stat(statbuf). Calls dirAction and optionally recurse on link to dir. + */ + int recursive_action(const char *fileName, int recurse, int followLinks, int depthFirst, - int (*fileAction) (const char *fileName, struct stat * statbuf, void* userData), - int (*dirAction) (const char *fileName, struct stat * statbuf, void* userData), - void* userData) + int (*fileAction)(const char *fileName, struct stat *statbuf, void* userData, int depth), + int (*dirAction)(const char *fileName, struct stat *statbuf, void* userData, int depth), + void* userData, + int depth) { struct stat statbuf; int status; @@ -53,21 +59,23 @@ int recursive_action(const char *fileName, return FALSE; } - if (!followLinks && (S_ISLNK(statbuf.st_mode))) { - return fileAction(fileName, &statbuf, userData); + /* If S_ISLNK(m), then we know that !S_ISDIR(m). + * Then we can skip checking first part: if it is true, then + * (!dir) is also true! */ + if ( /* (!followLinks && S_ISLNK(statbuf.st_mode)) || */ + !S_ISDIR(statbuf.st_mode) + ) { + return fileAction(fileName, &statbuf, userData, depth); } + /* It's a directory (or a link to one, and followLinks is set) */ + if (!recurse) { - if (S_ISDIR(statbuf.st_mode)) { - return dirAction(fileName, &statbuf, userData); - } + return dirAction(fileName, &statbuf, userData, depth); } - if (!S_ISDIR(statbuf.st_mode)) - return fileAction(fileName, &statbuf, userData); - if (!depthFirst) { - status = dirAction(fileName, &statbuf, userData); + status = dirAction(fileName, &statbuf, userData, depth); if (!status) { bb_perror_msg("%s", fileName); return FALSE; @@ -88,14 +96,14 @@ int recursive_action(const char *fileName, if (nextFile == NULL) continue; if (!recursive_action(nextFile, TRUE, followLinks, depthFirst, - fileAction, dirAction, userData)) { + fileAction, dirAction, userData, depth+1)) { status = FALSE; } free(nextFile); } closedir(dir); if (depthFirst) { - if (!dirAction(fileName, &statbuf, userData)) { + if (!dirAction(fileName, &statbuf, userData, depth)) { bb_perror_msg("%s", fileName); return FALSE; } diff --git a/modutils/insmod.c b/modutils/insmod.c index 00e25f5..49b823d 100644 --- a/modutils/insmod.c +++ b/modutils/insmod.c @@ -801,7 +801,7 @@ static char *m_fullName; static int check_module_name_match(const char *filename, struct stat *statbuf, - void *userdata) + void *userdata, int depth) { char *fullname = (char *) userdata; @@ -4048,7 +4048,7 @@ int insmod_main( int argc, char **argv) else module_dir = real_module_dir; recursive_action(module_dir, TRUE, FALSE, FALSE, - check_module_name_match, 0, m_fullName); + check_module_name_match, 0, m_fullName, 0); free(tmdn); } @@ -4063,7 +4063,7 @@ int insmod_main( int argc, char **argv) /* No module found under /lib/modules/`uname -r`, this * time cast the net a bit wider. Search /lib/modules/ */ if (!recursive_action(module_dir, TRUE, FALSE, FALSE, - check_module_name_match, 0, m_fullName) + check_module_name_match, 0, m_fullName, 0) ) { if (m_filename == 0 || ((fp = fopen(m_filename, "r")) == NULL) -- cgit v1.1