For something like 'mkdir -m 0700 foo', if the caller happens to have
a permissive umask (say allowing group write via 0007 or 0002), the
created directory will temporarily have more permissions than
requested. That's a mild security issue.
This reworks bb_make_directory() to always create both intermediate
and the final component with 0 permissions, then chmods to the final
value.
Aside from the robustification, this also simplifies the code
somewhat (net -31LOC), since we get rid of the complicated "what umask
do we have now and be sure to set it back" - we only need to query the
current umask in case mode==-1, and can set it back immediately. Then
mode contains the mode of the final component, and intermediate
components should then be chmod'ed to pmode=mode|0300.
x86_64 defconfig:
function old new delta
bb_make_directory 452 396 -56
Signed-off-by: Rasmus Villemoes <rasmus.villem...@prevas.dk>
---
Lightly tested and strace'd to see that it behaves as expected.
libbb/make_directory.c | 83 +++++++++++++-----------------------------
1 file changed, 26 insertions(+), 57 deletions(-)
diff --git a/libbb/make_directory.c b/libbb/make_directory.c
index 9b03bb8d0..9d95c2383 100644
--- a/libbb/make_directory.c
+++ b/libbb/make_directory.c
@@ -26,9 +26,8 @@
int FAST_FUNC bb_make_directory(char *path, long mode, int flags)
{
- mode_t cur_mask;
- mode_t org_mask;
const char *fail_msg;
+ mode_t pmode;
char *s;
char c;
struct stat st;
@@ -48,7 +47,13 @@ int FAST_FUNC bb_make_directory(char *path, long mode, int
flags)
// return 0; /* ".." */
}
- org_mask = cur_mask = (mode_t)-1L;
+ if (mode == -1) {
+ mode_t org_mask = umask(0);
+ mode = 0777 & ~org_mask;
+ umask(org_mask);
+ }
+ pmode = mode | 0300;
+
s = path;
while (1) {
c = '\0';
@@ -68,32 +73,8 @@ int FAST_FUNC bb_make_directory(char *path, long mode, int
flags)
}
}
- if (c != '\0') {
- /* Intermediate dirs: must have wx for user */
- if (cur_mask == (mode_t)-1L) { /* wasn't done yet? */
- mode_t new_mask;
- org_mask = umask(0);
- cur_mask = 0;
- /* Clear u=wx in umask - this ensures
- * they won't be cleared on mkdir */
- new_mask = (org_mask & ~(mode_t)0300);
- //bb_error_msg("org_mask:%o cur_mask:%o",
org_mask, new_mask);
- if (new_mask != cur_mask) {
- cur_mask = new_mask;
- umask(new_mask);
- }
- }
- } else {
- /* Last component: uses original umask */
- //bb_error_msg("1 org_mask:%o", org_mask);
- if (org_mask != cur_mask) {
- cur_mask = org_mask;
- umask(org_mask);
- }
- }
-
//bb_error_msg("mkdir '%s'", path);
- if (mkdir(path, 0777) < 0) {
+ if (mkdir(path, 0000) < 0) {
/* If we failed for any other reason than the directory
* already exists, output a diagnostic and return -1 */
if ((errno != EEXIST && errno != EISDIR)
@@ -103,36 +84,31 @@ int FAST_FUNC bb_make_directory(char *path, long mode, int
flags)
fail_msg = "create";
break;
}
- /* Since the directory exists, don't attempt to change
- * permissions if it was the full target. Note that
- * this is not an error condition. */
- if (!c) {
- goto ret0;
- }
+ /* Since the directory exists, don't attempt
+ * to change permissions. Note that this is
+ * not an error condition. */
} else {
if (flags & FILEUTILS_VERBOSE) {
printf("created directory: '%s'\n", path);
}
- }
-
- if (!c) {
- /* Done. If necessary, update perms on the newly
- * created directory. Failure to update here _is_
- * an error. */
- if (mode != -1) {
- //bb_error_msg("chmod 0%03lo mkdir '%s'", mode,
path);
- if (chmod(path, mode) < 0) {
- fail_msg = "set permissions of";
- if (flags & FILEUTILS_IGNORE_CHMOD_ERR)
{
- flags = 0;
- goto print_err;
- }
- break;
+ if (chmod(path, c ? pmode : mode) < 0) {
+ fail_msg = "set permissions of";
+ if (flags & FILEUTILS_IGNORE_CHMOD_ERR) {
+ flags = 0;
+ goto print_err;
}
+ break;
}
- goto ret0;
}
+ /* If this was the final component, we are done.
+ * Otherwise, continue with the next component, which
+ * may then fail to be created if the current one
+ * already existed but does not have sufficient
+ * permissions. */
+ if (!c)
+ return 0;
+
/* Remove any inserted nul from the path (recursive mode) */
*s = c;
} /* while (1) */
@@ -140,12 +116,5 @@ int FAST_FUNC bb_make_directory(char *path, long mode, int
flags)
flags = -1;
print_err:
bb_perror_msg("can't %s directory '%s'", fail_msg, path);
- goto ret;
- ret0:
- flags = 0;
- ret:
- //bb_error_msg("2 org_mask:%o", org_mask);
- if (org_mask != cur_mask)
- umask(org_mask);
return flags;
}
--
2.23.0
_______________________________________________
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
