Re: Xserver stack smashed -- wrapper

Pavel Kankovsky (peak@kerberos.troja.mff.cuni.cz)
Wed, 21 Jan 1998 21:58:55 +0100

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 ]