← Commit list

8c35d65c4

Denis Vlasenko
Denis Vlasenko
2006-10-27
FAILED Duration: 4s

Subject

recursive_action: add depth param chmod: match coreutils versus following links

Binary

No binary (build failed)

Architecture & Compiler Flags

Architecture: x86_64-linux-gnu Compiler: gcc (Ubuntu 11.4.0-1ubuntu1~22.04.3) 11.4.0

—

Build Log

(log yok)

Commit Diff GitHub →

archival/tar.c +2 -2coreutils/chmod.c +76 -22coreutils/chown.c +5 -3coreutils/diff.c +3 -3findutils/find.c +3 -3findutils/grep.c +3 -2include/libbb.h +4 -4libbb/recursive_action.c +24 -16modutils/insmod.c +3 -3
archival/tar.c+2 -2
@@ -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;
}
coreutils/chmod.c+76 -22
@@ -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"
+*/
coreutils/chown.c+5 -3
@@ -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;
}
coreutils/diff.c+3 -3
@@ -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);
}
findutils/find.c+3 -3
@@ -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;
}
}
findutils/grep.c+3 -2
@@ -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;
}
include/libbb.h+4 -4
@@ -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);
libbb/recursive_action.c+24 -16
@@ -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;
}
modutils/insmod.c+3 -3
@@ -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)