Description of problem:
The kernel allows processes to access the internal ".reiserfs_priv" directory at the top of a reiserfs filesystem which is used to store xattrs. Permissions are not enforced in that tree, so unprivileged users can view and potentially modify the xattrs on arbitrary files.
Version-Release number of selected component (if applicable):
Steps to Reproduce:
truncate --size 64M test.reiserfs
mkreiserfs -f test.reiserfs
mount -o loop,rw,user_xattr test.reiserfs /mnt/test
setfattr -n user.test -v myvalue /mnt/test
As an unprivileged user:
ls -l /mnt/test/.reiserfs_priv/xattrs/2.0
rm /mnt/test/.reiserfs_priv/xattrs/2.0/user.test # Whoops
The unprivileged user can read and write to /mnt/test/.reiserfs_priv .
All processes are denied access to /mnt/test/.reiserfs_priv .
I tested with SELinux completely disabled; I don't know if that makes a difference.
I don't know when the problem started. For comparison, when I booted to a Fedora 10 live image, the kernel correctly blocked access to .reiserfs_priv .
-rwx------ 1 root root 15 2010-03-01 19:52 user.test
kyle@phobos /mnt/test $ rm .reiserfs_priv/xattrs/2.0/user.test
kyle@phobos /mnt/test $
Looks like reiserfs xattr_unlink is missing a call to may_delete like vfs_unlink has. :/
Odd, I received an email with comment #4 but cannot see the comment here. Red Hat folks, is it really necessary to keep your comments private? Are you actually discussing embargoed security issues other than this one? Anyway...
(In reply to comment #4)
> -rwx------ 1 root root 15 2010-03-01 19:52 user.test
> kyle@phobos /mnt/test $ rm .reiserfs_priv/xattrs/2.0/user.test
> kyle@phobos /mnt/test $
> Looks like reiserfs xattr_unlink is missing a call to may_delete like
> vfs_unlink has. :/
Rather, permissions are not checked under /.reiserfs_priv because that tree is supposed to be accessed only by internal filesystem code, after permission checking has been done elsewhere. See:
Sorry, yeah, accidentally clicked the private thing. Fixed. It's a private bug for now anyway, so only the security guys, you, and people on the CC list can see it atm.
Ugh, reiserfs is a kludgy mess. I *think* what we want to do is check permissions during namei.c::reiserfs_lookup when checking IS_PRIVATE, and fail with -EACCES there if an ordinary user is attempting to bugger with the .reiserfs_priv dir. That should avoid screwing with the other proper xattr entrypoints.
This worked as recently as the Fedora 10 live CD. It might be worth understanding how the access check previously worked.
Yeah, I can see where it's broken (I think) but not how to rectify the breakage with the changes in the privroot handling. We'll see what Al says. I'm not really a filesystem person.
Ok, I think I've puzzled out the bit that needs fixing. Reverting the bits that allow _lookup to succeed on the privroot fixes the issue in my testing.
My analysis: xattr_lookup_poison is commented "This will catch lookups from the fs root to .reiserfs_priv", but all it actually does is prevent lookups from matching the dentry structure held in REISERFS_SB(s)->priv_root. So such lookups will just create a second dentry structure for .reiserfs_priv from the same dentry on disk! IIUC, the dentry structures for each directory are maintained in a hash table in its parent's dentry structure, so two parallel trees of dentry structures would result. This would explain the gross inconsistencies I saw when manipulating the privroot manually and running getfattr/setfattr at the same time.
The patch looks right to me. IIUC, we still need xattr_lookup_poison to prevent a lookup from matching REISERFS_SB(s)->priv_root via the cache before it even gets to reiserfs_lookup.
This issue did not affect the versions of Linux kernel as shipped with Red Hat Enterprise Linux 3, 4, 5 or Red Hat Enterprise MRG, as they do not include support for reiserfs.
I reported this to email@example.com . Adding Dann Frazier from the Debian security team so he can see the full discussion.
Kyle, any progress on this one?
Reported to CERT/CC (http://www.cert.org/), report ID VRF#G7I2H94M .
Edward is our local reiserfs expert - Edward, any suggestions about how to fix this for Fedora?
There is a proposed patch linked in comment #10, which I believe is correct but could use additional review.
I've asked Jeff Mahoney, the maintainer, to reconsider this commit
(677c9b2e393a0cd203bd54e9c18b012b2c73305a). The patch in comment #10 looks good to me and I would recommend it as a temporal fixup.
Thanks to everyone.
(In reply to comment #22)
> Yes, please share with as many vendors as you like so this issue can be fixed
> and disclosed without further delay!
Matt, please informed CERT/CC that this has been assigned with CVE-2010-1146. This issue will be public once the official fix is committed in the upstream kernel.
Edward, please update this bug when commit 677c9b2e3 is reconsidered. Also, please let us know when the fix is committed upstream, so that I can update other vendors. Thanks.
(In reply to comment #24)
> Matt, please informed CERT/CC that this has been assigned with CVE-2010-1146.
I have done so.
Jeff Mahoney suggested his own version of the fixup:
I've ack-ed this: I think in Jeff's version permissions are
checked more gracefully. At least reiserfs_lookup is critical
function, and it is important to keep it light-weight.
Considering this public:
The patch has been pushed to the upstream:
Fixed in 22.214.171.124-112
kernel-126.96.36.199-114.fc12 has been submitted as an update for Fedora 12.
kernel-188.8.131.52-115.fc12 has been submitted as an update for Fedora 12.
kernel-184.108.40.206-115.fc12 has been pushed to the Fedora 12 stable repository. If problems still persist, please make note of it in this bug report.