[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20110725230222.GB23791@openwall.com>
Date: Tue, 26 Jul 2011 03:02:22 +0400
From: Solar Designer <solar@...nwall.com>
To: Jeff Johnson <n3npq@....com>
Cc: oss-security@...ts.openwall.com
Subject: Re: CVE Request -- rpm -- Fails to remove the SUID/SGID bits on package upgrade (RH BZ#598775)
Jeff,
Thank you for your comments!
On Mon, Jul 25, 2011 at 03:39:15PM -0400, Jeff Johnson wrote:
> There were a series of CVE's applied (and some withdrawn) against
> whatever happens to be called "rpm".
>
> The patch here was dropped when RPM was forked and the CVE was
> essentially a replay of an issue that was already fixed 5 years ago
> (and the patch was NOT dropped in @rpm5.org cvs).
I am not sure I understand what you mean here. As I wrote, I am aware
of two CVEs relevant to the general issue: CVE-2005-4889 (package
removals) and CVE-2010-2059 (package upgrades). The corresponding
issues were in fact fixed in rpm4 at different times. Neither fix was
reverted in rpm4. Neither CVE id was withdrawn.
Are you saying that the fix for CVE-2005-4889 was somehow dropped from
rpm5, another CVE id was assigned, and the fix was re-introduced?
I have no idea - I am just trying to guess what you might have meant.
> (aside)
> I believe there are better fixes if the link count is more carefully
> checked always and everywhere. While rpm package metadata does not
> (and SHOULD not) carry an expected value for st->st_nlinks, its
> rather easy to synthesize an expected link count given the inode
> information (which is in rpm metadata) and to warn (either with --verify,
> or perhaps always) if the link count is not as expected.
Unfortunately, only doing the chmod() to safe perms if the link count is
other than expected is prone to a race condition. rpm4 actually had
this race condition introduced, then removed:
commit 89be57ad9239c9ada0cba94a5003876b456d46bf
Author: Panu Matilainen <pmatilai@...hat.com>
Date: Fri Jun 11 08:17:12 2010 +0300
If there are no hardlinks, dont bother with s-bit and caps removal
commit 26874707edfe73e153383284f9fe33cfd9879bb1
Author: Michal Schmidt <mschmidt@...hat.com>
Date: Tue Jun 22 15:51:41 2010 +0200
Revert "If there are no hardlinks, dont bother with s-bit and caps removal"
Deciding whether it is necessary to remove the SUID bit based on
the current link count creates an opportunity for a race condition.
A hardlink could be created just between lstat() and chmod().
This reverts commit 89be57ad9239c9ada0cba94a5003876b456d46bf.
> There are other (and better) approaches if the actual values on
> the file system, including files not contained in packages, is
> stored in an rpmdb: its a fundamental design flaw in RPM that
> only package metadata installed in an rpmdb is ever used
> for security auditing.
To me, system integrity checking for security purposes is mostly not a
package manager task, although sometimes it is in fact useful that rpm
can do it, even if to a very limited extent.
As to having rpm check/remove some files that are closely related to a
package being verified/removed but that didn't come from the package,
isn't this what %ghost is for? It won't verify those files' contents,
but I think that maintaining a database of hashes of changing files on a
system is not a package manager's task anyway.
> But there's no harm at all in removing SUID/SGID bits from files that are being
> removed in case there's an additional link that has been added.
Yes, and it has to be done regardless of link count (as long as we don't
have an atomic "chmod if st_nlink is ..." operation).
The purpose of my posting was to suggest that a similar cleanup is also
needed for things that are not SUID/SGID binaries, but also at least for
device files and for regular files with world or group write permissions.
Since it is not obvious if that list is exhaustive or not and since new
file types may appear later, I felt that chmod'ing all files to be
removed to 0 is a safer thing to do.
Of course, even that might not reset attributes stored outside of the
Unix permissions mask, such as fscaps. So those need to be taken care
of separately, which fscaps-aware builds of rpm4 already do. Perhaps
rpm5 does as well - I haven't looked yet.
The patch that I posted was against rpm 4.2, which was not fscaps-aware.
This is why I did not bother with that aspect of the issue there.
Thanks again,
Alexander
Powered by blists - more mailing lists
Please check out the
Open Source Software Security Wiki, which is counterpart to this
mailing list.
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
