Comment 9 for bug 1783591

On Thu, Jul 26, 2018 at 11:51:27AM -0000, Matthias Gerstner wrote:
> I have looked at the patch and security wise it looks very good to me.
> For completeness I would also add the O_CLOEXEC i.e.:
>
> netns_fd = open(args.pid, O_PATH | O_CLOEXEC);

The manpage (man 2 open ) states that:

"When O_PATH is specified in flags, flag bits other than O_CLOEXEC,
 O_DIRECTORY, and O_NOFOLLOW are ignored."

that's why I left it out.

>
> I think there is still a functional issue, however. You are using
> `fstatvfs()` and evaluate `f_fsid`. This is, as far as I understand, a
> unique identifier for file systems independent of their file system
> type. It returns zero for pseudo file systems.

Ok, I'll check the kernel sources but I need to switch this to our
has_fs_type() helper anyway that does:

/* __typeof__ should be safe to use with all compilers. */
typedef __typeof__(((struct statfs *)NULL)->f_type) fs_type_magic;
extern bool has_fs_type(const char *path, fs_type_magic magic_val);
extern bool is_fs_type(const struct statfs *fs, fs_type_magic magic_val);

bool is_fs_type(const struct statfs *fs, fs_type_magic magic_val)
{
        return (fs->f_type == (fs_type_magic)magic_val);
}

bool has_fs_type(const char *path, fs_type_magic magic_val)
{
        bool has_type;
        int ret;
        struct statfs sb;

        ret = statfs(path, &sb);
        if (ret < 0)
                return false;

        has_type = is_fs_type(&sb, magic_val);
        if (!has_type && magic_val == RAMFS_MAGIC)
                WARN("When the ramfs it a tmpfs statfs() might report tmpfs");

        return has_type;
}

Thanks!
Christian

>
> To get the file system magic you need to use `fstatfs()` and evaluate
> `f_type` instead.
>
> --
> You received this bug notification because you are a member of Ubuntu
> Container Security team, which is subscribed to the bug report.
> https://bugs.launchpad.net/bugs/1783591
>
> Title:
> lxc-user-nic allows unprivileged users to open arbitrary files
>
> Status in lxc package in Ubuntu:
> Triaged
> Status in lxc source package in Xenial:
> Triaged
> Status in lxc source package in Bionic:
> Triaged
> Status in lxc source package in Cosmic:
> Triaged
>
> Bug description:
> Matthias Gerstner from SUSE reported the following:
>
> ```
> Hello,
>
> following the lxc security reporting guidelines [1] I am reporting a
> finding in the lxc-user-nic setuid binary. I'm encrypting this mail as a
> best practice and because I found valid GPG keys for all of your
> adresses. Please find my public key attached to this mail.
>
> In the context of an openSUSE security audit of the lxc-user-nic setuid
> binary [2] (currently private bug) I came across an issue that should be
> adressed. In the "delete" case the program runs the following piece of
> code unconditionally with effective uid 0 (from lxc_user_nic.c):
>
> ```
> } else if (request == LXC_USERNIC_DELETE) {
> netns_fd = open(args.pid, O_RDONLY);
> if (netns_fd < 0) {
> usernic_error("Could not open \"%s\": %s\n", args.pid,
> strerror(errno));
> exit(EXIT_FAILURE);
> }
> }
> ```
>
> `args.pid` is a user controlled parameter and can be an arbitrary path
> at the moment. Nothing is done with this file descriptor later on in the
> program except an attempt at `setns(fd, CLONE_NEWNET)` in
> `is_privileged_over_netns()`. Still this allows the unprivileged caller
> of the setuid binary to achieve the following:
>
> - it can test for existence of files normally not accessible to the
> caller (information leak). Example:
> ```
> # this file is existing
> $ /usr/lib/lxc/lxc-user-nic delete path name /root/.bash_history type bridge nic
> lxc_user_nic.c: 1017: is_privileged_over_netns: Failed to setns() to network namespace Invalid argument
> lxc_user_nic.c: 1161: main: Process is not privileged over network namespace
>
> # this file is not existing
> $ /usr/lib/lxc/lxc-user-nic delete path name /root/.zsh_history type bridge nic
> lxc_user_nic.c: 1130: main: Could not open "/root/.zsh_history": No such file or directory
> ```
>
> - it allows to trigger code paths in the kernel that are normally not
> accessible to the caller. This can happen when opening special files
> like character and block devices or files in /proc or /sys. Opening
> some of these files can cause lock or alloc operations or even more
> complex things to happen like when opening /dev/ptmx, which causes the
> allocation of a new master/slave pseudo terminal. Therefore this can
> lead to DoS like situations or have further unspecified impact.
>
> For fixing this I suggest opening the file supplied in `args.pid` only
> with the permissions of the real user, since this is already done in
> `is_privileged_over_netns()` anyway. Another approach would be the
> normalization of the input path and then only allowing a path of the
> pattern /proc/<pid>/ns/net.
>
> [1] https://github.com/lxc/lxc/blob/master/README.md#reporting-security-issues
> [2] https://bugzilla.suse.com/show_bug.cgi?id=988348
>
> Best regards
>
> Matthias
> ```
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/ubuntu/+source/lxc/+bug/1783591/+subscriptions