Bug 7116 - setuid return fixes
setuid return fixes
Status: RESOLVED FIXED
Product: xorg
Classification: Unclassified
Component: * Other
git
All Linux (All)
: high normal
Assigned To: Matthieu Herrb
: security
: 18797 18798 18799 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2006-06-05 03:21 UTC by Matthieu Herrb
Modified: 2008-12-01 08:24 UTC (History)
5 users (show)

See Also:
i915 platform:
i915 features:


Attachments
patch for X.Org 6.9 (13.57 KB, patch)
2006-06-05 03:45 UTC, Matthieu Herrb
no flags Details | Splinter Review
patch for CVS HEAD (8.26 KB, patch)
2006-06-05 04:19 UTC, Matthieu Herrb
no flags Details | Splinter Review
patch for GIT HEAD (libX11) (420 bytes, patch)
2006-06-05 04:20 UTC, Matthieu Herrb
no flags Details | Splinter Review
libX11 1.0.1 patch (420 bytes, patch)
2006-06-19 14:41 UTC, Matthieu Herrb
no flags Details | Splinter Review
xdm 1.0.4 patch (1.56 KB, patch)
2006-06-19 14:41 UTC, Matthieu Herrb
no flags Details | Splinter Review
xf86dga 1.0.1 patch (709 bytes, patch)
2006-06-19 14:42 UTC, Matthieu Herrb
no flags Details | Splinter Review
xinit 1.0.2 patch (721 bytes, patch)
2006-06-19 14:42 UTC, Matthieu Herrb
no flags Details | Splinter Review
xload 1.0.1 patch (1.07 KB, patch)
2006-06-19 14:43 UTC, Matthieu Herrb
no flags Details | Splinter Review
xserver 1.1.0 patch (3.52 KB, patch)
2006-06-19 14:44 UTC, Matthieu Herrb
no flags Details | Splinter Review
xtrans 1.0.0 patch (900 bytes, patch)
2006-06-19 14:44 UTC, Matthieu Herrb
no flags Details | Splinter Review
update X11R6.9.0 patch (13.57 KB, patch)
2006-06-19 14:45 UTC, Matthieu Herrb
no flags Details | Splinter Review
X 6.8.2 patch (12.57 KB, patch)
2006-06-19 14:46 UTC, Matthieu Herrb
no flags Details | Splinter Review
Proposed advisory text (2.70 KB, text/plain)
2006-06-19 14:47 UTC, Matthieu Herrb
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Matthieu Herrb 2006-06-05 03:21:35 UTC
Fixes for setuid return values checks
Comment 1 Matthieu Herrb 2006-06-05 03:45:15 UTC
Created attachment 5818 [details] [review]
patch for X.Org 6.9
Comment 2 Matthieu Herrb 2006-06-05 04:19:42 UTC
Created attachment 5819 [details] [review]
patch for CVS HEAD
Comment 3 Matthieu Herrb 2006-06-05 04:20:26 UTC
Created attachment 5820 [details] [review]
patch for GIT HEAD (libX11)
Comment 4 Matthieu Herrb 2006-06-05 04:21:44 UTC
I've added patches for both monolithic and modular trees. 
Comment 5 ajax at nwnk dot net 2006-06-05 08:09:19 UTC
is this a real bug or just defensive programming?
Comment 6 Matthieu Herrb 2006-06-05 09:22:06 UTC
(In reply to comment #5)
> is this a real bug or just defensive programming?

There are some real bugs. Here's Marcus Meissner original message to xorg_security:

Hi,

Dirk Mueller and I have been checking for setuid/seteuid without return
checks.

One offender who is exploitable we think is X. Both X.Org and XFree86.

For the readers who just joined a brief summary:

  In kernel 2.6 it is possible that setuid(user_uid); can fail even
  if done from root a process.

  Reason is that there is the maximum processes "ulimit" which is 
  honoured by setuid(), seteuid(), set*uid().
  If you do not check the return value and continue as-is you have
  not dropped the privilege, but run as root.

  An example of this has been released in "vixie-cron".

  Since ulimits on maximum processes are set by the kernel by default,
  any Linux 2.6 system is default affected.


  Fix: Check the return value of setuid() and handle it and/or
  check with getuid() right after the setuid() if it worked.

My evaluation might be off and it might not be exploitable in these
places.

If this is exploitable I would propose an embargo of 2 weeks.
(Tue June 20th 12:00 UTC would be a nice date)



Critical MUST FIX problems:
./programs/Xserver/hw/xfree86/common/xf86Init.c:          setuid(getuid());
	Security relevant and critical.
	After this "sh -c "vtinit"" is called, which might be exploitable.

	(SUSE has the X Server setuid root, so this would affect us.)

./programs/Xserver/hw/xfree86/parser/write.c:			setuid(getuid());
        Security relevant and critical.
	Can corrupt any file on the system, like /etc/shadow.
	(I think via: Xorg -configure --... /etc/shadow)


./programs/Xserver/hw/xfree86/os-support/shared/libc_wrapper.c:       
setuid(getuid());
        Security relevant and critical.
	In xf86execl(), which I do not know who calls it.

./programs/Xserver/os/utils.c:	setuid(getuid());
./programs/Xserver/os/utils.c:	setuid(getuid());
./programs/Xserver/os/utils.c:	setuid(getuid());
        Security relevant and critical.
	These are in Popen(), Fopen(), System().

	Leak file content, start called programs as root.

All of them above are in the XServer.

Normal problems, but should be fixed probably:

./lib/X11/lcFile.c:	    if (seteuid(0) != 0) {
./lib/X11/lcFile.c:		seteuid(oldeuid);
	Obscure.

./lib/xtrans/Xtranslcl.c:	setuid( getuid() ); /** sets the euid to the
actual/real uid **/
	Obscure, not used in modern systems.

./config/util/chownxterm.c:    setuid(getuid());
./config/util/chownxterm.c:    setuid(getuid());
	No setuid/setgid xterm used, we have utempter and devpts.

./programs/xdm/session.c:	setuid (verify.uid);
	This needs to be fixed, but is not security critical.

	It is called before the KRB5 tickets are dropped
	and the user auth is removed.

./programs/xdm/xdmshell.c:    setuid (geteuid());
	Should be fixed. In common use scenarios it is not setuid-root,
	so not critical.

./programs/xinit/xinit.c:		setuid(getuid());
	Security relevant.
	Must be fixed. This is critical when xinit is setuid-root,
	but I do not know if anyone has it so.

./programs/xload/xload.c:    setuid(getuid());
	Security relevant.
	No one ships xload setuid root anymore, but it should be fixed.

./programs/xterm/main.c:    seteuid(getuid());
./programs/xterm/main.c:    setuid(getuid());
./programs/xterm/main.c:    seteuid(getuid());
./programs/xterm/main.c:    setuid(getuid());
./programs/xterm/main.c:	seteuid(0);
./programs/xterm/main.c:	seteuid(getuid());
./programs/xterm/main.c:	setuid(screen->uid);
./programs/xterm/main.c:	    if (setuid(screen->uid)) {
./programs/xterm/misc.c:	setuid(uid);
./programs/xterm/misc.c:	    setuid(screen->uid);
./programs/xterm/os2main.c:	setuid(screen->uid);
./programs/xterm/os2main.c:	    setuid(screen->uid);
./programs/xterm/print.c:	    setuid(screen->uid);
	No one ships xterm setuid root anymore, but it should be fixed
	for those who do.

./programs/xf86dga/dga.c:   setuid(getuid());
	The old DGA binary, not shipped setuid root at SUSE.
	It does not do much exploitable afterwards, but it usually
	destroys your X session anyway, so it is better to not run it.


Ciao, Marcus

_______________________________________________
Xorg_security mailing list
Xorg_security@x.org
http://expo.x.org/mailman/listinfo/xorg_security
Comment 7 Alan Coopersmith 2006-06-14 13:34:18 UTC
For xdm, you'll probably want to patch/release the xdm-1_0-branch, which is the
stable release used in Xorg releases so far.   I've been making bigger changes in
HEAD for a 1.1 release which isn't quite ready for release yet.

For this problem as a whole, Solaris does not allow setuid() to fail in this
fashion, so we don't believe we will need to release fixes for Solaris for this,
so don't worry about us in coordinating the release.
Comment 8 Matthieu Herrb 2006-06-19 12:47:37 UTC
Should we prepare an advisory and release patches, or just commit them to -current? 
Afaict, there's no CVE-ID for this issue. 
Comment 9 Matthieu Herrb 2006-06-19 14:41:08 UTC
Created attachment 5973 [details] [review]
libX11 1.0.1 patch
Comment 10 Matthieu Herrb 2006-06-19 14:41:55 UTC
Created attachment 5974 [details] [review]
xdm 1.0.4 patch
Comment 11 Matthieu Herrb 2006-06-19 14:42:27 UTC
Created attachment 5975 [details] [review]
xf86dga 1.0.1 patch
Comment 12 Matthieu Herrb 2006-06-19 14:42:55 UTC
Created attachment 5976 [details] [review]
xinit 1.0.2 patch
Comment 13 Matthieu Herrb 2006-06-19 14:43:25 UTC
Created attachment 5977 [details] [review]
xload 1.0.1 patch
Comment 14 Matthieu Herrb 2006-06-19 14:44:05 UTC
Created attachment 5978 [details] [review]
xserver 1.1.0 patch
Comment 15 Matthieu Herrb 2006-06-19 14:44:44 UTC
Created attachment 5979 [details] [review]
xtrans 1.0.0 patch
Comment 16 Matthieu Herrb 2006-06-19 14:45:35 UTC
Created attachment 5980 [details] [review]
update X11R6.9.0 patch
Comment 17 Matthieu Herrb 2006-06-19 14:46:07 UTC
Created attachment 5981 [details] [review]
X 6.8.2 patch
Comment 18 Matthieu Herrb 2006-06-19 14:47:41 UTC
Created attachment 5983 [details]
Proposed advisory text
Comment 19 Daniel Stone 2006-06-19 15:09:21 UTC
proposed new body.  all grammatical, except for one editorial change: 'in some
corner cases', end of first paragraph.  if this looks good, please post out to
vendor-sec, xorg, etc, later on today.  also to xorg-announce@, but you need
[ANNOUNCE] as the start of the subject for that one.

X.Org Security Advisory, June 20th, 2006
setuid return value check problems on Linux systems

Overview

A lack of checks for setuid() failures when invoked by a privileged
process (e.g., X server, xdm, xterm, if installed setuid or setgid)
may cause the process to execute certain privileged operations
(file access) as root while it was intended to be executed with a
less privileged effective user ID, on systems where setuid() called
by root can fail.  This can be used by a malicious local user to
overwrite files and possibly elevate privileges in some corner
cases.

Vulnerability details

In Linux 2.6, it is possible that setuid(user_uid). can fail even
when invoked from a process running as root.

This is because there is a 'maximum processes' ulimit, which is
honoured by setuid(), seteuid(), and setgid().
These functions may fail because of this ulimit; if the return
value is not checked, then code which is assumed to be running
unprivileged, may in fact be running with uid 0.

Since ulimits on maximum processes are set by the kernel by default,
any Linux 2.6 system is affected by default..

Affected versions

X.Org versions 6.7.0 to 7.1 inclusive are vulnerable on systems
where setuid() called by root may fail. Older X11R6 versions are
probably affected also, but are not supported by X.Org.
Comment 20 Matthieu Herrb 2006-06-20 02:28:32 UTC
Note: I fixed some buglets in the final patches (removed one leftover debug
printf and RCS ID). So the versions that I'm uploading to annarchy and the
corresponding hashes will be different.
Comment 21 Daniel Stone 2006-06-20 06:51:32 UTC
open the floodgates
Comment 22 Matthieu Herrb 2006-06-20 12:20:16 UTC
All patches commited to HEAD of git/CVS repositories.
Comment 23 Alan Coopersmith 2008-12-01 08:23:45 UTC
*** Bug 18797 has been marked as a duplicate of this bug. ***
Comment 24 Alan Coopersmith 2008-12-01 08:24:05 UTC
*** Bug 18798 has been marked as a duplicate of this bug. ***
Comment 25 Alan Coopersmith 2008-12-01 08:24:17 UTC
*** Bug 18799 has been marked as a duplicate of this bug. ***