> Security bug: mkdir removes the sticky bit
>
> There is a security bug in mkdir:
>
> > mkdir -m 1777 /tmp/foo
> > ls -l /tmp
> drwxrwxrwx 2 root root 1024 May 14 19:44 foo
>
> The sticky bit is missing.
>
> > strace mkdir -m 1777 /tmp/foo |& grep mkdir
> mkdir("/tmp/foo", 01777) = 0
>
> So it looks to be really kernel related.
Yup, it is. Patch follows:
diff -urN linux-2.3.2/fs/ChangeLog linux-bird.mkdir/fs/ChangeLog
--- linux-2.3.2/fs/ChangeLog Thu May 13 07:02:54 1999
+++ linux-bird.mkdir/fs/ChangeLog Sun May 16 02:33:54 1999
@@ -144,24 +144,12 @@
* rmdir of immutable/append-only directory shouldn't be allowed. Fixed.
Remains unfixed:
- * UMSDOS_rename is broken. Call it with the dest. existing and being an
- empty directory and you've got EBUSY. At least it doesn't do
- any harm, so that will wait several days till rename cleanup.
- Sigh... It will wait a bit more. Problems with fat-derived
- filesystems are much worse than I thought. Idea of changing
- inode under dentry is broken by design - guess where the
- semaphore sits, for one.
- * umsdos: weird. rename() shouldn't return -EEXIST. BTW, manpage
- for rename(2) is obviously bogus - it mentions EEXIST and
- on the next line (correctly) says that EINVAL should be
- returned. Under the same conditions.
* rename's handling of races is, erm, not optimal. Looks like I know
what to do, but this thing needs some more cleanup - we can
take care of almost all races in VFS and be much more graceful
wrt locking. Moreover, it would give strong lookup atomicity.
But it's a lot of changes to lookup and dcache code, so it will
go after the fs drivers' cleanup.
- * hfs allows mknod. Only for regular files ;-/ IMHO it's bogus.
* affs allows HARD links to directories. VFS is, to put it politely,
not too ready to cope with _that_. And I'm not sure it should
be - looks like they are pretty much similar to symlinks.
@@ -169,8 +157,3 @@
braindead filesystems). I've submitted a patch to Linus, but
looks like it wasn't applied.
* msdos: shouldn't we treat SYS as IMMUTABLE? Makes sense, IMHO.
- * minix, qnx and sysv do NOT allow to mkdir sticky directories.
- * {minix,sysv}/namei.c (do_{minix,syv}_{rename,unlink}):
- Stuff related to retries still needs cleanup/fixing.
- Looks like I've found an extremely low-probability race
- there...
diff -urN linux-2.3.2/fs/affs/namei.c linux-bird.mkdir/fs/affs/namei.c
--- linux-2.3.2/fs/affs/namei.c Thu May 13 07:02:55 1999
+++ linux-bird.mkdir/fs/affs/namei.c Sun May 16 02:30:04 1999
@@ -315,7 +315,7 @@
error = affs_add_entry(dir,NULL,inode,dentry,ST_USERDIR);
if (error)
goto out_iput;
- inode->i_mode = S_IFDIR | S_ISVTX | (mode & 0777 & ~current->fs->umask);
+ inode->i_mode = S_IFDIR | S_ISVTX | mode;
inode->u.affs_i.i_protect = mode_to_prot(inode->i_mode);
d_instantiate(dentry,inode);
mark_inode_dirty(inode);
diff -urN linux-2.3.2/fs/ext2/namei.c linux-bird.mkdir/fs/ext2/namei.c
--- linux-2.3.2/fs/ext2/namei.c Thu May 13 23:03:17 1999
+++ linux-bird.mkdir/fs/ext2/namei.c Sun May 16 02:27:54 1999
@@ -490,7 +490,7 @@
inode->i_nlink = 2;
mark_buffer_dirty(dir_block, 1);
brelse (dir_block);
- inode->i_mode = S_IFDIR | (mode & (S_IRWXUGO|S_ISVTX) & ~current->fs->umask);
+ inode->i_mode = S_IFDIR | mode;
if (dir->i_mode & S_ISGID)
inode->i_mode |= S_ISGID;
mark_inode_dirty(inode);
diff -urN linux-2.3.2/fs/minix/namei.c linux-bird.mkdir/fs/minix/namei.c
--- linux-2.3.2/fs/minix/namei.c Thu May 13 23:03:18 1999
+++ linux-bird.mkdir/fs/minix/namei.c Sun May 16 02:25:31 1999
@@ -297,7 +297,7 @@
inode->i_nlink = 2;
mark_buffer_dirty(dir_block, 1);
brelse(dir_block);
- inode->i_mode = S_IFDIR | (mode & 0777 & ~current->fs->umask);
+ inode->i_mode = S_IFDIR | mode;
if (dir->i_mode & S_ISGID)
inode->i_mode |= S_ISGID;
mark_inode_dirty(inode);
diff -urN linux-2.3.2/fs/namei.c linux-bird.mkdir/fs/namei.c
--- linux-2.3.2/fs/namei.c Thu May 13 23:03:18 1999
+++ linux-bird.mkdir/fs/namei.c Sun May 16 02:19:42 1999
@@ -882,10 +882,6 @@
return error;
}
-/*
- * Look out: this function may change a normal dentry
- * into a directory dentry (different size)..
- */
static inline int do_mkdir(const char * pathname, int mode)
{
int error;
@@ -919,7 +915,7 @@
goto exit_lock;
DQUOT_INIT(dir->d_inode);
- mode &= 0777 & ~current->fs->umask;
+ mode &= (S_IRWXUGO|S_ISVTX) & ~current->fs->umask;
error = dir->d_inode->i_op->mkdir(dir->d_inode, dentry, mode);
exit_lock:
diff -urN linux-2.3.2/fs/nfsd/vfs.c linux-bird.mkdir/fs/nfsd/vfs.c
--- linux-2.3.2/fs/nfsd/vfs.c Thu May 13 07:03:02 1999
+++ linux-bird.mkdir/fs/nfsd/vfs.c Sun May 16 02:19:37 1999
@@ -691,6 +691,8 @@
break;
case S_IFDIR:
opfunc = (nfsd_dirop_t) dirp->i_op->mkdir;
+ /* Odd, indeed, but filesystems did it anyway */
+ iap->ia_mode &= (S_IRWXUGO|S_ISVTX) & ~current->fs->umask;
break;
case S_IFCHR:
case S_IFBLK:
diff -urN linux-2.3.2/fs/sysv/namei.c linux-bird.mkdir/fs/sysv/namei.c
--- linux-2.3.2/fs/sysv/namei.c Thu May 13 23:03:19 1999
+++ linux-bird.mkdir/fs/sysv/namei.c Sun May 16 02:21:07 1999
@@ -291,7 +291,7 @@
inode->i_nlink = 2;
mark_buffer_dirty(dir_block, 1);
brelse(dir_block);
- inode->i_mode = S_IFDIR | (mode & 0777 & ~current->fs->umask);
+ inode->i_mode = S_IFDIR | mode;
if (dir->i_mode & S_ISGID)
inode->i_mode |= S_ISGID;
mark_inode_dirty(inode);
diff -urN linux-2.3.2/fs/ufs/namei.c linux-bird.mkdir/fs/ufs/namei.c
--- linux-2.3.2/fs/ufs/namei.c Thu May 13 23:03:19 1999
+++ linux-bird.mkdir/fs/ufs/namei.c Sun May 16 02:22:02 1999
@@ -539,7 +539,7 @@
inode->i_nlink = 2;
mark_buffer_dirty(dir_block, 1);
brelse (dir_block);
- inode->i_mode = S_IFDIR | (mode & (S_IRWXUGO|S_ISVTX) & ~current->fs->umask);
+ inode->i_mode = S_IFDIR | mode;
if (dir->i_mode & S_ISGID)
inode->i_mode |= S_ISGID;
mark_inode_dirty(inode);
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/