Bug 7116

Created attachment 5818 [details] [review]
patch for X.Org 6.9

Created attachment 5819 [details] [review]
patch for CVS HEAD

Created attachment 5820 [details] [review]
patch for GIT HEAD (libX11)

I've added patches for both monolithic and modular trees. 

is this a real bug or just defensive programming?

(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

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.

Should we prepare an advisory and release patches, or just commit them to -current? 
Afaict, there's no CVE-ID for this issue. 

Created attachment 5973 [details] [review]
libX11 1.0.1 patch

Created attachment 5974 [details] [review]
xdm 1.0.4 patch

Created attachment 5975 [details] [review]
xf86dga 1.0.1 patch

Created attachment 5976 [details] [review]
xinit 1.0.2 patch

Created attachment 5977 [details] [review]
xload 1.0.1 patch

Created attachment 5978 [details] [review]
xserver 1.1.0 patch

Created attachment 5979 [details] [review]
xtrans 1.0.0 patch

Created attachment 5980 [details] [review]
update X11R6.9.0 patch

Created attachment 5981 [details] [review]
X 6.8.2 patch

Created attachment 5983 [details]
Proposed advisory text

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.

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.

open the floodgates

All patches commited to HEAD of git/CVS repositories.

*** Bug 18797 has been marked as a duplicate of this bug. ***

*** Bug 18798 has been marked as a duplicate of this bug. ***

*** Bug 18799 has been marked as a duplicate of this bug. ***