recursive_action: add depth param

chmod: match coreutils versus following links

3 files changed, 84 insertions, 28 deletions

diff --git a/coreutils/chmod.c b/coreutils/chmod.c
index c4f8fa0b2..b601504f8 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 bef89ce86..fddce7cf1 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 2915d4009..f26bcca86 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);
 	}