]> git.ipfire.org Git - thirdparty/shadow.git/commitdiff
copy_tree: carefully treat permissions
authorSamanta Navarro <ferivoz@riseup.net>
Sat, 10 Sep 2022 11:58:15 +0000 (11:58 +0000)
committerIker Pedrosa <ikerpedrosam@gmail.com>
Wed, 14 Sep 2022 08:11:32 +0000 (10:11 +0200)
The setuid, setgid, and sticky bits are not copied during copy_tree.

Also start with very restrictive permissions before setting ownerships.

This prevents situations in which users in a group with less permissions
than others could win a race in opening the file before permissions are
removed again.

Proof of concept:

$ echo $HOME
/home/uwu
$ install -o uwu -g fandom -m 604 /dev/null /home/uwu/owo
$ ls -l /home/uwu/owo
-rw----r-- 1 uwu fandom 0 Sep  4 00:00 /home/uwu/owo

If /tmp is on another filesystem, then "usermod -md /tmp/uwu uwu" leads
to this temporary situation:

$ ls -l /tmp/uwu/owo
-rw----r-- 1 root root  0 Sep  4 00:00 /tmp/uwu/owo

This means that between openat and chownat_if_needed a user of group
fandom could open /tmp/uwu/owo and read the content when it is finally
written into the file.

libmisc/copydir.c

index 5fb47da01e4b1f733854b430f7cfdd7fc0845fbe..24dfd5902a762556a4fcfa5fb8488b2464251c78 100644 (file)
@@ -522,15 +522,14 @@ static int copy_dir (const struct path_info *src, const struct path_info *dst,
                return -1;
        }
 #endif                         /* WITH_SELINUX */
-       if (   (mkdirat (dst->dirfd, dst->name, statp->st_mode) != 0)
+       if (   (mkdirat (dst->dirfd, dst->name, 0700) != 0)
            || (chownat_if_needed (dst, statp,
                                 old_uid, new_uid, old_gid, new_gid) != 0)
+           || (fchmodat (dst->dirfd, dst->name, statp->st_mode & 07777, AT_SYMLINK_NOFOLLOW) != 0)
 #ifdef WITH_ACL
            || (   (perm_copy_path (src, dst, &ctx) != 0)
                && (errno != 0))
-#else                          /* !WITH_ACL */
-           || (fchmodat (dst->dirfd, dst->name, statp->st_mode & 07777, AT_SYMLINK_NOFOLLOW) != 0)
-#endif                         /* !WITH_ACL */
+#endif                         /* WITH_ACL */
 #ifdef WITH_ATTR
        /*
         * If the third parameter is NULL, all extended attributes
@@ -719,12 +718,11 @@ static int copy_special (const struct path_info *src, const struct path_info *ds
        if (   (mknodat (dst->dirfd, dst->name, statp->st_mode & ~07777U, statp->st_rdev) != 0)
            || (chownat_if_needed (dst, statp,
                                 old_uid, new_uid, old_gid, new_gid) != 0)
+           || (fchmodat (dst->dirfd, dst->name, statp->st_mode & 07777, AT_SYMLINK_NOFOLLOW) != 0)
 #ifdef WITH_ACL
            || (   (perm_copy_path (src, dst, &ctx) != 0)
                && (errno != 0))
-#else                          /* !WITH_ACL */
-           || (fchmodat (dst->dirfd, dst->name, statp->st_mode & 07777, AT_SYMLINK_NOFOLLOW) != 0)
-#endif                         /* !WITH_ACL */
+#endif                         /* WITH_ACL */
 #ifdef WITH_ATTR
        /*
         * If the third parameter is NULL, all extended attributes
@@ -810,16 +808,15 @@ static int copy_file (const struct path_info *src, const struct path_info *dst,
                return -1;
        }
 #endif                         /* WITH_SELINUX */
-       ofd = openat (dst->dirfd, dst->name, O_WRONLY | O_CREAT | O_EXCL | O_TRUNC | O_NOFOLLOW | O_CLOEXEC, statp->st_mode & 07777);
+       ofd = openat (dst->dirfd, dst->name, O_WRONLY | O_CREAT | O_EXCL | O_TRUNC | O_NOFOLLOW | O_CLOEXEC, 0600);
        if (   (ofd < 0)
            || (fchown_if_needed (ofd, statp,
                                  old_uid, new_uid, old_gid, new_gid) != 0)
+           || (fchmod (ofd, statp->st_mode & 07777) != 0)
 #ifdef WITH_ACL
            || (   (perm_copy_fd (src->full_path, ifd, dst->full_path, ofd, &ctx) != 0)
                && (errno != 0))
-#else                          /* !WITH_ACL */
-           || (fchmod (ofd, statp->st_mode & 07777) != 0)
-#endif                         /* !WITH_ACL */
+#endif                         /* WITH_ACL */
 #ifdef WITH_ATTR
        /*
         * If the third parameter is NULL, all extended attributes