[6025] in bugtraq
Re: Xserver stack smashed -- wrapper
daemon@ATHENA.MIT.EDU (Pavel Kankovsky)
Wed Jan 21 16:42:05 1998
Date: Wed, 21 Jan 1998 21:58:55 +0100
Reply-To: peak@kerberos.troja.mff.cuni.cz
From: Pavel Kankovsky <peak@KERBEROS.TROJA.MFF.CUNI.CZ>
To: BUGTRAQ@NETSPACE.ORG
In-Reply-To: <r6z4t2x3dvs.fsf@peridot.southwind.net>
On Wed, 21 Jan 1998, John Goerzen wrote:
> A short time ago, there was some talk about various wrappers around
> the X server, and I pointed out that Debian already has one better
> than the example posted. Since then, I have received requests to post
> Debian's wrapper source.
Unfortunately, this wrapper has two serious flaws:
> case Console:
> if (fstat(0,&s)!=0) {
> fprintf(stderr,"X: cannot stat stdin\n");
> return FALSE;
> }
> if (S_ISCHR(s.st_mode) && ((s.st_rdev>>8)&0xff)==VT_MAJOR_DEV &&
> (s.st_rdev&0xff)<128) {
> return TRUE;
> }
> break;
First flaw: it is quite easy to fool this check. In many cases, it is
possible to find a world writable vc entry in /dev (yes, this is a kind
of configuration error but AFAIK Debian itself ships with a load of world
writable /dev/tty[0-9]*'s) and do this:
int
main()
{
close(0);
open("/dev/tty0", O_WRONLY);
execlp("xserver-wrapper", "xserver-wrapper", 0);
}
IMHO, /var/run/utmp ought to be consulted
> for (i = 1; i < argc; i++) {
> if (!strcmp(argv[i], "-config")) {
> if (setuid(getuid())) {
> perror("X couldn't drop setuid privileges for alternate config");
> exit(1);
> }
> break;
> }
> }
> execv(xserver,argv);
Second flaw: not paranoid enough when checking the arguments.
It should test whether arguments are _allowed_ and their parameters
have _sane_ values.
--Pavel Kankovsky aka Peak (troja.mff.cuni.cz network administration)
[ Boycott Microsoft -- http://www.vcnet.com/bms ]
