On Fri, Feb 24, 2017 at 4:23 PM, Eric Blake <ebl...@redhat.com> wrote: > On 02/24/2017 04:34 AM, Greg Kurz wrote: >> On Thu, 23 Feb 2017 15:10:42 +0000 >> Stefan Hajnoczi <stefa...@gmail.com> wrote: >> >>> On Mon, Feb 20, 2017 at 03:42:19PM +0100, Greg Kurz wrote: >>>> The local_chmod() callback is vulnerable to symlink attacks because it >>>> calls: >>>> >>>> (1) chmod() which follows symbolic links for all path elements >>>> (2) local_set_xattr()->setxattr() which follows symbolic links for all >>>> path elements >>>> (3) local_set_mapped_file_attr() which calls in turn local_fopen() and >>>> mkdir(), both functions following symbolic links for all path >>>> elements but the rightmost one >>>> > >>>> + fd = local_open_nofollow(fs_ctx, fs_path->data, O_RDONLY, 0); >>>> + if (fd == -1) { >>>> + return -1; >>>> + } >>>> + ret = fchmod(fd, credp->fc_mode); >>>> + close_preserve_errno(fd); >>> >>> As mentioned on IRC, not sure this is workable since chmod(2) must not >>> require read permission on the file. This patch introduces failures >>> when the file isn't readable. >>> >> >> Yeah :-\ This one is the more painful issue to address. I need a >> chmod() variant that would fail on symlinks instead of following >> them... and I'm kinda suffering to implement this in a race-free >> manner. > > BSD has lchmod(), but Linux does not. And Linux does not yet properly > implement AT_SYMLINK_NOFOLLOW. (The BSD semantics are pretty cool: > restricting mode bits on a symlink can create scenarios where some users > can dereference the symlink but others get ENOENT failures - but I don't > know of any effort to add those semantics to the Linux kernel) > > POSIX says support for AT_SYMLINK_NOFOLLOW is optional, precisely > because POSIX does not require permissions on symlinks to matter, but > the way it requires it is that AT_SYMLINK_NOFOLLOW is supposed to be a > no-op for regular files, and cause an EOPNOTSUPP failure for symlinks. > > Unfortunately, the Linux man page says that Linux fails with ENOTSUP > (which is identical to EOPNOTSUPP) any time AT_SYMLINK_NOFOLLOW is set, > and not just if it is attempted on a symlink.
And unfortunately, that flags argument is not actually present in the real syscall. See this glibc code: int fchmodat (int fd, const char *file, mode_t mode, int flag) { if (flag & ~AT_SYMLINK_NOFOLLOW) return INLINE_SYSCALL_ERROR_RETURN_VALUE (EINVAL); #ifndef __NR_lchmod /* Linux so far has no lchmod syscall. */ if (flag & AT_SYMLINK_NOFOLLOW) return INLINE_SYSCALL_ERROR_RETURN_VALUE (ENOTSUP); #endif return INLINE_SYSCALL (fchmodat, 3, fd, file, mode); } and this kernel code: SYSCALL_DEFINE3(fchmodat, int, dfd, const char __user *, filename, umode_t, mode) { [...] } So to fix this, you'll probably have to add a new syscall fchmodat2() to the kernel, wire it up for all the architectures and get the various libc implementations to adopt that. That's going to be quite tedious. :(