[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: extended attributes security problem



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.

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.

> Not allowing them for symlinks could be a problem for some other non ACL
> uses of EAs (e.g. if a GUI fs browser wanted to store an icon in there), but 
> this is probably not a too big problem right now. 
> 
> Of course this makes the existence of l{get,list,remove}attr a bit
> questionable, but then at least root can do something with them still.
> 
> -Andi
> 
> 
> --- linux-work/fs/xattr.c-o	Thu Mar 21 18:15:26 2002
> +++ linux-work/fs/xattr.c	Sun Apr  7 13:03:06 2002
> @@ -67,6 +67,11 @@
>  	if (flags & ~(XATTR_CREATE|XATTR_REPLACE))
>  		return -EINVAL;
>  
> +	/* Do not allow creation of EAs on special files and symlinks. */
> +	if (!S_ISREG(d->d_inode->i_mode)  && !S_ISDIR(d->d_inode->i_mode) &&
> +		!capable(CAP_SYS_ADMIN))	
> +		return -EPERM; 
> +
>  	error = strncpy_from_user(kname, name, sizeof(kname));
>  	if (error == 0 || error == sizeof(kname))
>  		error = -ERANGE;

-- 
Ethan Benson
http://www.alaska.net/~erbenson/

Attachment: pgp00006.pgp
Description: PGP signature