Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Tue, 26 Sep 2017 16:40:31 +0200
From: Jann Horn <jannh@...gle.com>
To: Salvatore Mesoraca <s.mesoraca16@...il.com>
Cc: kernel list <linux-kernel@...r.kernel.org>, 
	Kernel Hardening <kernel-hardening@...ts.openwall.com>, Kees Cook <keescook@...omium.org>, 
	Solar Designer <solar@...nwall.com>, Alexander Viro <viro@...iv.linux.org.uk>, 
	"Eric W. Biederman" <ebiederm@...ssion.com>, linux-fsdevel@...r.kernel.org
Subject: Re: [RFC v2 2/2] Protected O_CREAT open in sticky directory

On Tue, Sep 26, 2017 at 4:14 PM, Salvatore Mesoraca
<s.mesoraca16@...il.com> wrote:
> Disallows O_CREAT open missing the O_EXCL flag, in world or
> group writable directories, even if the file doesn't exist yet.
> With few exceptions (e.g. shared lock files based on flock())
> if a program tries to open a file with the O_CREAT flag and
> without the O_EXCL, it probably has a bug.
> This feature allows to detect and potentially block programs that
> act this way and can be used to find vulnerabilities (like those
> prevented by patch #1) and to do policy enforcement.
[...]
> +static int may_create_no_excl(struct dentry * const dir,
> +                             const unsigned char * const name,
> +                             struct inode * const inode)
> +{
> +       umode_t mode = dir->d_inode->i_mode;
> +
> +       if (likely(!(mode & S_ISVTX)))
> +               return 0;
> +       if (inode && (uid_eq(inode->i_uid, dir->d_inode->i_uid) ||
> +                     uid_eq(current_fsuid(), inode->i_uid)))
> +               return 0;

Why is there an exception for inode->i_uid==dir->d_inode->i_uid? Is this
kind of behavior more likely to be correct when the directory owner is
doing it?


> +       if ((sysctl_protected_sticky_child_create && likely(mode & 0002)) ||
> +           (sysctl_protected_sticky_child_create >= 2 && mode & 0020)) {
> +               pr_notice_ratelimited("unsafe O_CREAT open (missing O_EXCL) of '%s' in a sticky directory by UID %d, EUID %d, process %s:%d.\n",
> +                                     name,
> +                                     from_kuid(&init_user_ns, current_uid()),
> +                                     from_kuid(&init_user_ns, current_euid()),
> +                                     current->comm, current->pid);

As far as I can tell, uid_t is unsigned, so the first two "%d"
format string elements are technically wrong:
  extern uid_t from_kuid(struct user_namespace *to, kuid_t uid);
  typedef __kernel_uid32_t uid_t;
  typedef unsigned int __kernel_uid32_t;

> +               if (sysctl_protected_sticky_child_create >= 4 ||
> +                   (sysctl_protected_sticky_child_create == 3 &&
> +                    likely(mode & 0002)))
> +                       return -EACCES;
> +       }
> +       return 0;
> +}

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.