Openwall GNU/*/Linux - a small security-enhanced Linux distro for servers
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Mon, 25 Jul 2011 06:08:27 +0400
From: Solar Designer <solar@...nwall.com>
To: Jan Lieskovsky <jlieskov@...hat.com>
Cc: oss-security <oss-security@...ts.openwall.com>,
	Panu Matilainen <pmatilai@...hat.com>,
	Jindrich Novy <jnovy@...hat.com>, Florian Festi <ffesti@...hat.com>,
	Matt McCutchen <matt@...tmccutchen.net>,
	yersinia <yersinia.spiros@...il.com>
Subject: Re: CVE Request -- rpm -- Fails to remove the SUID/SGID bits on package upgrade (RH BZ#598775)

Hi,

I know I am being a year late to comment on this, but maybe better late
than never.  I'll over-quote a little bit:

On Wed, Jun 02, 2010 at 01:43:03PM +0200, Jan Lieskovsky wrote:
>    Matt McCutchen pointed out a deficiency in the way rpm handled rpm 
>    package upgrades --
> it failed to clear out the SUID/SGID bits of the old file by file 
> replacement when privileged
> user performed package upgrade. Under certain circumstances, a local, 
> authenticated user could
> use this flaw to escalate their privileges.
> 
> Red Hat Bugzilla entry:
>   [1] https://bugzilla.redhat.com/show_bug.cgi?id=598775
> 
> Upstream changeset:
>   [2] 
>   http://rpm.org/gitweb?p=rpm.git;a=commit;h=ca2d6b2b484f1501eafdde02e1688409340d2383
> 
> Could you allocate CVE id for this?

This was assigned CVE-2010-2059.  And there was a similar issue 5 years
earlier, affecting package removals (rather than upgrades):

https://bugzilla.redhat.com/show_bug.cgi?id=125517

That one was assigned CVE-2005-4889.

The descriptions and fixes for these were limited to SUID/SGID bits and
fscaps (on RPM versions/builds supporting those).  However, what about
device files, maybe with unsafe permissions (or just extra device files
that would need to be gone on package removal or on upgrade to a package
version no longer providing those)?  What about regular files with world
or group write permissions, which allow for a quota bypass?

Additionally, the fix for SUID/SGID bits ignores the return value from
chmod(), so it is fail-open.  It would not even print an error message.
It is unclear what the best action on a partially-failed old package
removal would be, but at the very least we can print an error message.

Sure, these are relatively minor issues, yet if we're fixing things, we
could as well fix them more fully.

What I am considering is chmod()'ing everything but symlinks to 0, as
opposed to the current fix of removing SUID/SGID bits off regular files
with those bits set only.

I've attached a patch against rpm 4.2 (yes, ancient code), which passed
my basic tests (but more testing is needed).

I don't care to request a third CVE id for this; I merely want to
include a better fix.

I'd appreciate any comments.

Thanks,

Alexander

diff -urp rpm-4.2.orig/lib/fsm.c rpm-4.2/lib/fsm.c
--- rpm-4.2.orig/lib/fsm.c	2003-03-03 19:38:32 +0000
+++ rpm-4.2/lib/fsm.c	2011-07-25 01:31:24 +0000
@@ -1990,26 +1990,54 @@ if (!(fsm->mapFlags & CPIO_ALL_HARDLINKS
 	/*@...reached@*/ break;
 
     case FSM_UNLINK:
-	rc = Unlink(fsm->path);
+	{
+	    struct stat stb;
+	    int saved_errno;
+	    int saved_rc = lstat(fsm->path, &stb);
+	    if (!saved_rc && !S_ISLNK(stb.st_mode))
+		saved_rc = chmod(fsm->path, 0);
+	    saved_errno = errno;
+	    if (saved_rc && saved_errno == ENOENT)
+		saved_rc = 0;
+	    rc = Unlink(fsm->path);
+	    if (!rc && saved_rc) {
+		rc = saved_rc;
+		errno = saved_errno;
+	    }
+	}
 	if (_fsm_debug && (stage & FSM_SYSCALL))
 	    rpmMessage(RPMMESS_DEBUG, " %8s (%s) %s\n", cur,
 		fsm->path, (rc < 0 ? strerror(errno) : ""));
 	if (rc < 0)	rc = CPIOERR_UNLINK_FAILED;
 	break;
     case FSM_RENAME:
-	rc = Rename(fsm->opath, fsm->path);
+	{
+	    struct stat stb;
+	    int saved_errno;
+	    int saved_rc = lstat(fsm->path, &stb);
+	    if (!saved_rc && !S_ISLNK(stb.st_mode))
+		saved_rc = chmod(fsm->path, 0);
+	    saved_errno = errno;
+	    if (saved_rc && saved_errno == ENOENT)
+		saved_rc = 0;
+	    rc = Rename(fsm->opath, fsm->path);
 #if defined(ETXTBSY)
-	if (rc && errno == ETXTBSY) {
-	    char * path = alloca(strlen(fsm->path) + sizeof("-RPMDELETE"));
-	    (void) stpcpy( stpcpy(path, fsm->path), "-RPMDELETE");
-	    /*
-	     * XXX HP-UX (and other os'es) don't permit rename to busy
-	     * XXX files.
-	     */
-	    rc = Rename(fsm->path, path);
-	    if (!rc) rc = Rename(fsm->opath, fsm->path);
-	}
+	    if (rc && errno == ETXTBSY) {
+		char * path = alloca(strlen(fsm->path) + sizeof("-RPMDELETE"));
+		(void) stpcpy( stpcpy(path, fsm->path), "-RPMDELETE");
+		/*
+		* XXX HP-UX (and other os'es) don't permit rename to busy
+		* XXX files.
+		*/
+		rc = Rename(fsm->path, path);
+		if (!rc) rc = Rename(fsm->opath, fsm->path);
+	    }
 #endif
+	    if (!rc && saved_rc) {
+		rc = saved_rc;
+		errno = saved_errno;
+	    }
+	}
 	if (_fsm_debug && (stage & FSM_SYSCALL))
 	    rpmMessage(RPMMESS_DEBUG, " %8s (%s, %s) %s\n", cur,
 		fsm->opath, fsm->path, (rc < 0 ? strerror(errno) : ""));

Powered by blists - more mailing lists

Your e-mail address:

Please check out the Open Source Software Security Wiki, which is counterpart to this mailing list.

Powered by Openwall GNU/*/Linux - Powered by OpenVZ