Messages in this thread | | | Subject | Re: [PATCH 2/2] fs: Harden against open(..., O_CREAT, 02777) in a setgid directory | From | Ben Hutchings <> | Date | Wed, 25 Jan 2017 21:31:59 +0000 |
| |
On Wed, 2017-01-25 at 13:06 -0800, Andy Lutomirski wrote: > Currently, if you open("foo", O_WRONLY | O_CREAT | ..., 02777) in a > directory that is setgid and owned by a different gid than current's > fsgid, you end up with an SGID executable that is owned by the > directory's GID. This is a Bad Thing (tm). Exploiting this is > nontrivial because most ways of creating a new file create an empty > file and empty executables aren't particularly interesting, but this > is nevertheless quite dangerous. > > Harden against this type of attack by detecting this particular > corner case (unprivileged program creates SGID executable inode in > SGID directory owned by a different GID) and clearing the new > inode's SGID bit. > > > Signed-off-by: Andy Lutomirski <luto@kernel.org> > --- > fs/inode.c | 21 +++++++++++++++++++-- > 1 file changed, 19 insertions(+), 2 deletions(-) > > diff --git a/fs/inode.c b/fs/inode.c > index f7029c40cfbd..d7e4b80470dd 100644 > --- a/fs/inode.c > +++ b/fs/inode.c > @@ -2007,11 +2007,28 @@ void inode_init_owner(struct inode *inode, const struct inode *dir, > { > inode->i_uid = current_fsuid(); > if (dir && dir->i_mode & S_ISGID) { > + bool changing_gid = !gid_eq(inode->i_gid, dir->i_gid); [...]
inode->i_gid hasn't been initialised yet. This should compare with current_fsgid(), shouldn't it?
Ben.
-- Ben Hutchings It is easier to write an incorrect program than to understand a correct one.
[unhandled content-type:application/pgp-signature] | |