DragonFlyBSD

:Hi.
:On HAMMER filesystem, you can remove an object you do not own even
:when the directory containing it has sticky bit set:

Oops!  I'll get that one fixed today, thanks for the report!

-Matt

:Hi.
:On HAMMER filesystem, you can remove an object you do not own even
:when the directory containing it has sticky bit set:

Committed as a kernel-layer fix.  Please test also, and if it works
    as expected I will cherry-pick it over to 2.2.x as well.

-Matt

On Sat, May 02, 2009 at 10:53:31AM -0700, Matthew Dillon wrote:

:Hi.
:On HAMMER filesystem, you can remove an object you do not own even
:when the directory containing it has sticky bit set:

Committed as a kernel-layer fix. Please test also, and if it works
as expected I will cherry-pick it over to 2.2.x as well.

Quoting from sticky(8):
... A file in a sticky directory may only be removed or renamed
by a user if the user has write permission for the directory and the user
is the owner of the file, the owner of the directory, or the super-user.

So rename(2) in a directory with sticky bit set should also be taken care of.
Currently there are three commits to be cherry-picked to 2.2 branch:
918101d3be679a76c85ea8f47b94e05f8eb11ee5
ee89633d5330733056052c06919a5dd4c01347bc
dae8d54f0708cf191cbb06ef6aa43cd570ceea90

The filesystems affected by ee89633d are: ext2fs, hpfs, msdosfs, ntfs,
nwfs, smbfs, udf, ufs, and HAMMER, but I only tested ufs and HAMMER.

And a very silly nit-pick would be that with these commits the return
code is changed from EPERM to EACCES. I can't think of a situation
when that could be a problem, though.

:Quoting from sticky(8):
: ... A file in a sticky directory may only be removed or renamed
: by a user if the user has write permission for the directory and the user
: is the owner of the file, the owner of the directory, or the super-user.
:
:So rename(2) in a directory with sticky bit set should also be taken care of.
:Currently there are three commits to be cherry-picked to 2.2 branch:
: 918101d3be679a76c85ea8f47b94e05f8eb11ee5
: ee89633d5330733056052c06919a5dd4c01347bc
: dae8d54f0708cf191cbb06ef6aa43cd570ceea90
:
:The filesystems affected by ee89633d are: ext2fs, hpfs, msdosfs, ntfs,
:nwfs, smbfs, udf, ufs, and HAMMER, but I only tested ufs and HAMMER.

Is rename(2) working properly?  I wasn't quite sure from your posting.
    It does seem to work properly in my quick test.

:And a very silly nit-pick would be that with these commits the return
:code is changed from EPERM to EACCES. I can't think of a situation
:when that could be a problem, though.
:

I don't mind synchronizing the error code with linux, if someone wants
    to figure out what linux returns in these situations.

-Matt
                    Matthew Dillon 
                    <dillon@backplane.com>

[sorry, I was talking about an unrelated thing here]
:

Is rename(2) working properly? I wasn't quite sure from your posting.
It does seem to work properly in my quick test.

The problem is I (as a non-root user) can still rename(2) an object that
I do not own, in a sticky directory on HAMMER filesystem.

1. mkdir -m 1777 sticky && touch sticky/ROOT
2. su -m nobody -c 'cd sticky && mv ROOT R00T && echo HEY'
   mv: rename ROOT to R00T: Operation not permitted

1. mkdir -m 1777 sticky && touch sticky/ROOT
2. su -m nobody -c 'cd sticky && mv ROOT R00T && echo HEY'
   HEY

:The problem is I (as a non-root user) can still rename(2) an object that
:I do not own, in a sticky directory on HAMMER filesystem.
:
:On UFS:
:# mkdir -m 1777 sticky && touch sticky/ROOT
:# su -m nobody -c 'cd sticky && mv ROOT R00T && echo HEY'
:mv: rename ROOT to R00T: Operation not permitted
:
:On HAMMER:
:# mkdir -m 1777 sticky && touch sticky/ROOT
:# su -m nobody -c 'cd sticky && mv ROOT R00T && echo HEY'
:HEY

test28# su -m nobody -c 'cd sticky && mv ROOT R00T2 && echo HEY'
    mv: rename ROOT to R00T2: No such file or directory
    test28#

There may be a short cut for renaming a file to itself, which is
    basically a NOP.  I'll track down the path.

-Matt
                    Matthew Dillon 
                    <dillon@backplane.com>

On Sun, May 03, 2009 at 08:27:46PM -0700, Matthew Dillon wrote:

:The problem is I (as a non-root user) can still rename(2) an object that
:I do not own, in a sticky directory on HAMMER filesystem.
:
:On UFS:
:# mkdir -m 1777 sticky && touch sticky/ROOT
:# su -m nobody -c 'cd sticky && mv ROOT R00T && echo HEY'
:mv: rename ROOT to R00T: Operation not permitted
:
:On HAMMER:
:# mkdir -m 1777 sticky && touch sticky/ROOT
:# su -m nobody -c 'cd sticky && mv ROOT R00T && echo HEY'
:HEY

test28# su -m nobody -c 'cd sticky && mv ROOT R00T2 && echo HEY'
mv: rename ROOT to R00T2: No such file or directory
test28#

This is expected, because the original file(ARE OH OH TEE) has been
renamed to (ARE ZERO ZERO TEE). I guess ZEROs and OHs are not very
distictive on your monitor :)

There may be a short cut for renaming a file to itself, which is
basically a NOP. I'll track down the path.

-Matt
Matthew Dillon
<dillon@backplane.com>

:This is expected, because the original file(ARE OH OH TEE) has been
:renamed to (ARE ZERO ZERO TEE). I guess ZEROs and OHs are not very
:distictive on your monitor :)

Nope!  Ok, you could have used something more obvious there :-)

In anycase, I've tracked it down.   The rename code was not checking
    the sticky bit for the rename target at all, so it happily overwrite
    the file owned by someone else.  It was doing a VDELETE test for the
    rename source (which tests the sticky bit) and a VCREATE test for the
    rename target (which is designed for O_CREAT and doesn't deal with
    rename's file replacement, so it wasn't).  I added a VRENAME and
    NLC_RENAME so I could properly code testing of the rename-target.

I'll commit a fix in a few minutes once I finish testing it.  

-Matt
                    Matthew Dillon 
                    <dillon@backplane.com>

The problem is even worse. rename() doesn't check directory perms
at all for the source directory on a HAMMER filesystem (because HAMMER
depends on the kernel to make the checks and doesn't do them itself).
I blew it.

That's a huge gaping security hole so we are going to have to
    roll a 2.2.2 this week to correct it.

-Matt
                    Matthew Dillon 
                    <dillon@backplane.com>

Fixed by dae8d54f0708 and 945b476ab592