Re: Xserver stack smashed -- wrapper

[peak () kerberos troja mff cuni cz (Pavel Kankovsky)]

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 ]