From cde221b8587193f9dc300c0799a530e846c75961 Mon Sep 17 00:00:00 2001 From: Samanta Navarro Date: Sat, 10 Sep 2022 11:58:15 +0000 Subject: [PATCH] copy_tree: carefully treat permissions 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 | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/libmisc/copydir.c b/libmisc/copydir.c index 5fb47da0..24dfd590 100644 --- a/libmisc/copydir.c +++ b/libmisc/copydir.c @@ -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