Comment 2 for bug 1894980

Revision history for this message
Christian Brauner (cbrauner) wrote :

Afaict, the bug is due to permission checking for copying up files in overlayfs that we are susceptible too because we allow unpriv overlayfs mounts.

When overlayfs does not support metacopy only copy up then overlayfs will copy up the whole file, i.e. including its contents, to the upper filesystem.

To this end overlayfs will call dentry_open() on any file from the lower filesystem in ovl_copy_up_flags(). But dentry_open() doesn't perform any permission checks and will always succeed.

After that overlayfs will copy the lower file contents into the upper filesystem and finally call ovl_set_attr() on it to change the mode and ids of the copied up file. Should ovl_set_attr() fail then the created file will have been improperly copied up, i.e. it will not accurately reflect the ownership the lower file had in the lower filesystem. An attacker can abuse this to get read access to a file on the system.

Getting ovl_set_attr() to fail can e.g. be triggered by mounting fuse-overlayfs in a new user and mount namespaces, then mounting shiftfs on top of it. Finally an overlayfs mount can be created on top of shiftfs that uses a directory or filesystem with invalid ids in the current user namespace as lower filesystem. Then a copy needs to be triggered in the overlayfs mount by e.g. renaming or moving a file in the merged directory. The initial copy up of the data of the file will succeed while the ovl_set_attr() will fail since the ids of the lower filesystem are not valid in the current user namespace. With some trickery, this can be used to leave a copied-up file with the caller's permissions lying in the workdir of the overlayfs mount. This file can now be read by the attacker.

Fixing this involves backporting

commit 56230d956739b9cb1cbde439d76227d77979a04d
Author: Miklos Szeredi <email address hidden>
Date: Tue Jun 2 22:20:26 2020 +0200

    ovl: verify permissions in ovl_path_open()

    Check permission before opening a real file.

    ovl_path_open() is used by readdir and copy-up routines.

    ovl_permission() theoretically already checked copy up permissions, but it
    doesn't hurt to re-do these checks during the actual copy-up.

    For directory reading ovl_permission() only checks access to topmost
    underlying layer. Readdir on a merged directory accesses layers below the
    topmost one as well. Permission wasn't checked for these layers.

    Note: modifying ovl_permission() to perform this check would be far more
    complex and hence more bug prone. The result is less precise permissions
    returned in access(2). If this turns out to be an issue, we can revisit
    this bug.

    Signed-off-by: Miklos Szeredi <email address hidden>

and a small number of preliminary patches since this issue has been fixed upstream.