From 076d7cc09eccff3d01d153434d89455db375ac44 Mon Sep 17 00:00:00 2001 From: markus schnalke Date: Fri, 23 Mar 2012 09:43:17 +0100 Subject: [PATCH] makedir(): Removed the strange RUID!=EUID code and the access(2) call. Dan couldn't find a reason for the code neither, hence I assume it's save to simply remove it. Ummm, why do we want to avoid creating directories with the effective user ID? None of the nmh tools are installed such that the effective should be different from the real, and if some parent process made the two be different, I don't see why it should be our job to enforce the real UID. Also, why the heck do we call the mkdir executable rather than the library function in this case?? If we do want to call the mkdir executable, we should at least be giving it -p (and change the single chmod() call below) so it can successfully create nested directories like the above code can. -- Dan Harkless Concerning access(2): We should better always just try the action instead of first asking if we are allowed to do it. --- sbr/makedir.c | 76 ++++++++++++--------------------------------------------- 1 file changed, 15 insertions(+), 61 deletions(-) diff --git a/sbr/makedir.c b/sbr/makedir.c index f064f77..a76f910 100644 --- a/sbr/makedir.c +++ b/sbr/makedir.c @@ -22,7 +22,6 @@ makedir(char *dir) char* folder_perms_ASCII; int had_an_error = 0; mode_t folder_perms, saved_umask; - pid_t pid; register char* c; context_save(); /* save the context file */ @@ -49,72 +48,27 @@ makedir(char *dir) */ saved_umask = umask(0); - if (getuid() == geteuid()) { - c = strncpy(path, dir, sizeof(path)); + c = strncpy(path, dir, sizeof(path)); - while (!had_an_error && (c = strchr((c + 1), '/')) != NULL) { - *c = (char)0; - if (access(path, X_OK)) { - if (errno != ENOENT){ - advise(dir, "unable to create directory"); - had_an_error = 1; - } - /* Create an outer directory. */ - if (mkdir(path, folder_perms)) { - advise(dir, "unable to create directory"); - had_an_error = 1; - } - } - *c = '/'; - } - - /* - ** Create the innermost nested subdirectory of the - ** path we're being asked to create. - */ - if (!had_an_error && mkdir(dir, folder_perms)==-1) { + while (!had_an_error && (c = strchr((c + 1), '/')) != NULL) { + *c = '\0'; + /* Create an outer directory. */ + if (mkdir(path, folder_perms) == -1 && + errno != EEXIST) { advise(dir, "unable to create directory"); had_an_error = 1; } - } else { - /* - ** Ummm, why do we want to avoid creating directories - ** with the effective user ID? None of the nmh tools are - ** installed such that the effective should be different - ** from the real, and if some parent process made the two - ** be different, I don't see why it should be our job to - ** enforce the real UID. Also, why the heck do we call - ** the mkdir executable rather than the library function in - ** this case?? If we do want to call the mkdir executable, - ** we should at least be giving it -p (and change the single - ** chmod() call below) so it can successfully create nested - ** directories like the above code can. - ** -- Dan Harkless - */ - switch (pid = fork()) { - case -1: - advise("fork", "unable to"); - return 0; - - case 0: - setgid(getgid()); - setuid(getuid()); - - execl("/bin/mkdir", "mkdir", dir, NULL); - execl("/usr/bin/mkdir", "mkdir", dir, NULL); - fprintf(stderr, "unable to exec "); - perror("mkdir"); - _exit(-1); - - default: - if (pidXwait(pid, "mkdir")) - return 0; - break; - } - - chmod(dir, folder_perms); + *c = '/'; } + /* + ** Create the innermost nested subdirectory of the + ** path we're being asked to create. + */ + if (!had_an_error && mkdir(dir, folder_perms)==-1) { + advise(dir, "unable to create directory"); + had_an_error = 1; + } umask(saved_umask); /* put the user's umask back */ return (had_an_error) ? 0 : 1; -- 1.7.10.4