xfs
[Top] [All Lists]

Re: extended attributes security problem

To: Ethan Benson <erbenson@xxxxxxxxxx>
Subject: Re: extended attributes security problem
From: Andi Kleen <ak@xxxxxxx>
Date: Sun, 7 Apr 2002 16:50:32 +0200
Cc: Andi Kleen <ak@xxxxxxx>, Andreas Gruenbacher <ag@xxxxxxxxxxx>, linux-xfs@xxxxxxxxxxx
In-reply-to: <20020407035631.L1524@plato.local.lan>
References: <20020405234103.F1524@plato.local.lan> <Pine.LNX.4.33.0204061724480.12984-100000@muriel.parsec.at> <20020406161039.J1524@plato.local.lan> <20020407131619.A13788@wotan.suse.de> <20020407035631.L1524@plato.local.lan>
Sender: owner-linux-xfs@xxxxxxxxxxx
User-agent: Mutt/1.3.22.1i
On Sun, Apr 07, 2002 at 03:56:31AM -0800, Ethan Benson wrote:
> On Sun, Apr 07, 2002 at 01:16:19PM +0200, Andi Kleen wrote:
> > 
> > I'm proposing this patch. As Andreas pointed out it doesn't make much sense
> > to set ACLs on symlinks or special devices. I still allow it for root.
> 
> sounds reasonable.
> 
> however this patch is not sufficent, we must do something about world
> writable directories like /tmp, otherwise my exploit will still work
> fine, just target /tmp instead of /dev/null.

In /tmp you can create files anyways and they're usually not even counted 
to your quota (at least few sites seem to run quota in /tmp) 

> 
> i think that restricting creation of user.* attrs to the owner when
> the sticky bit is set is really the only sensible solution here, i
> agree its not ideal but as its been stated the meaning of the sticky
> bit is a bit of a hack anyway.

Hmm. Ok. Revised patch.  Also do not allow setting on sticky bit.


--- linux-work/fs/xattr.c-o     Thu Mar 21 18:15:26 2002
+++ linux-work/fs/xattr.c       Sun Apr  7 16:41:15 2002
@@ -63,10 +63,22 @@
        int error;
        void *kvalue;
        char kname[XATTR_NAME_MAX + 1];
+       struct inode *i = d->d_inode; 
 
        if (flags & ~(XATTR_CREATE|XATTR_REPLACE))
                return -EINVAL;
 
+       /* Some checks to prevent people abusing EAs to get over their quota: 
*/ 
+       /* Do not allow creation of EAs on special files and symlinks */
+       if (!S_ISREG(i->i_mode)  && !S_ISDIR(i->i_mode) &&          
+               !capable(CAP_SYS_ADMIN))        
+               return -EPERM; 
+       /* Do not allow creation EAs in directories with sticky bit unless 
you're
+          the owner. */           
+       if (S_ISDIR(i->i_mode) && (i->i_mode & S_ISVTX) && 
+           (current->fsuid != i->i_uid) && !capable(CAP_FOWNER))
+               return -EPERM; 
+
        error = strncpy_from_user(kname, name, sizeof(kname));
        if (error == 0 || error == sizeof(kname))
                error = -ERANGE;
@@ -83,9 +95,9 @@
        }
 
        error = -EOPNOTSUPP;
-       if (d->d_inode->i_op && d->d_inode->i_op->setxattr) {
+       if (i->i_op && i->i_op->setxattr) {
                lock_kernel();
-               error = d->d_inode->i_op->setxattr(d, kname, kvalue, size, 
flags);
+               error = i->i_op->setxattr(d, kname, kvalue, size, flags);
                unlock_kernel();
        }
 




-Andi

<Prev in Thread] Current Thread [Next in Thread>