[1252] in bugtraq
safe logging xterm
daemon@ATHENA.MIT.EDU (Margarita Suarez)
Tue Mar 14 15:49:09 1995
Date: Tue, 14 Mar 95 12:55:58 EST
From: Margarita Suarez <marg@columbia.edu>
To: bugtraq@fc.net
Cc: unixsys@columbia.edu
Cert Advisory #CA-93:17 points out a vulnerability of xterm which can be
exploited when logging is turned on. X11R6 is distributed with logging
turned off.
we have modified xterm to make use of the POSIX saved id where possible;
otherwise, it uses setreuid() to switch back and forth between user and
superuser. we provide enable() and disable() functions which swap the
euid and ruid so that the running xterm can give up root and take it
back.
in the beginning of main, we disable, and only enable before opening the
tty or utmp file (disabling immediately after). we have verified that
before the log file is opened, xterm is disabled (running as the user,
not root).
i am appending the context diffs to the X11R6 source to Imakefile,
main.c, and misc.c. compile with DEBUG2 defined to see that xterm is
not root when the log file is opened. the Imakefile defines
ALLOWLOGGING and SAFESETUID (our mods). xterm is installed setuid by
default in X11R6 (defined in Project.tmpl), so TEST THIS AT YOUR OWN RISK.
can anyone see a problem with this fix?
Margarita Suarez
marg@columbia.edu
*** /tmp/,RCSt1019396 Tue Mar 14 12:49:21 1995
--- Imakefile Tue Mar 14 12:24:28 1995
***************
*** 27,32 ****
--- 27,33 ----
-DOSMAJORVERSION=$(OSMAJORVERSION) \
-DOSMINORVERSION=$(OSMINORVERSION)
MISC_DEFINES = /* -DALLOWLOGFILEEXEC */
+ DEFINES = -DALLOWLOGGING -DSAFESETUID
SRCS1 = button.c charproc.c cursor.c data.c input.c \
main.c menu.c misc.c screen.c scrollbar.c tabs.c \
*** /tmp/,RCSt1019396 Tue Mar 14 12:49:22 1995
--- main.c Tue Mar 14 12:42:06 1995
***************
*** 823,828 ****
--- 823,832 ----
Atom wm_delete_window;
+ static int Xterm_euid = -1;
+ static int Xterm_ruid = -1;
+
+
main (argc, argv)
int argc;
char **argv;
***************
*** 833,838 ****
--- 837,847 ----
char *base_name();
int xerror(), xioerror();
+ Xterm_ruid = getuid();
+ Xterm_euid = geteuid();
+
+ disable();
+
ProgramName = argv[0];
ttydev = (char *) malloc (strlen (TTYDEV) + 1);
***************
*** 1292,1298 ****
return 0;
#endif /* SYSV && SYSV386 */
#ifdef ATT
! if ((*pty = open ("/dev/ptmx", O_RDWR)) < 0) {
return 1;
}
#if defined(SVR4) || defined(SYSV386)
--- 1301,1307 ----
return 0;
#endif /* SYSV && SYSV386 */
#ifdef ATT
! if ((*pty = enable_open ("/dev/ptmx", O_RDWR)) < 0) {
return 1;
}
#if defined(SVR4) || defined(SYSV386)
***************
*** 1304,1310 ****
return 0;
#else /* ATT else */
#ifdef AIXV3
! if ((*pty = open ("/dev/ptc", O_RDWR)) < 0) {
return 1;
}
strcpy(ttydev, ttyname(*pty));
--- 1313,1319 ----
return 0;
#else /* ATT else */
#ifdef AIXV3
! if ((*pty = enable_open ("/dev/ptc", O_RDWR)) < 0) {
return 1;
}
strcpy(ttydev, ttyname(*pty));
***************
*** 1314,1320 ****
{
char *tty_name;
! tty_name = _getpty (pty, O_RDWR, 0622, 0);
if (tty_name == 0)
return 1;
strcpy (ttydev, tty_name);
--- 1323,1329 ----
{
char *tty_name;
! tty_name = _getpty (pty, O_RDWR, 0622, 0); /* xxx - can't enable */
if (tty_name == 0)
return 1;
strcpy (ttydev, tty_name);
***************
*** 1326,1332 ****
char *pty_name, *getpty();
while ((pty_name = getpty()) != NULL) {
! if ((*pty = open (pty_name, O_RDWR)) >= 0) {
strcpy(ptydev, pty_name);
strcpy(ttydev, pty_name);
ttydev[5] = 't';
--- 1335,1341 ----
char *pty_name, *getpty();
while ((pty_name = getpty()) != NULL) {
! if ((*pty = enable_open (pty_name, O_RDWR)) >= 0) {
strcpy(ptydev, pty_name);
strcpy(ttydev, pty_name);
ttydev[5] = 't';
***************
*** 1342,1348 ****
#if (defined(sgi) && OSMAJORVERSION < 4) || (defined(umips) && defined (SYSTYPE_SYSV))
struct stat fstat_buf;
! *pty = open ("/dev/ptc", O_RDWR);
if (*pty < 0 || (fstat (*pty, &fstat_buf)) < 0) {
return(1);
}
--- 1351,1357 ----
#if (defined(sgi) && OSMAJORVERSION < 4) || (defined(umips) && defined (SYSTYPE_SYSV))
struct stat fstat_buf;
! *pty = enable_open ("/dev/ptc", O_RDWR);
if (*pty < 0 || (fstat (*pty, &fstat_buf)) < 0) {
return(1);
}
***************
*** 1349,1355 ****
sprintf (ttydev, "/dev/ttyq%d", minor(fstat_buf.st_rdev));
#ifndef sgi
sprintf (ptydev, "/dev/ptyq%d", minor(fstat_buf.st_rdev));
! if ((*tty = open (ttydev, O_RDWR)) < 0) {
close (*pty);
return(1);
}
--- 1358,1364 ----
sprintf (ttydev, "/dev/ttyq%d", minor(fstat_buf.st_rdev));
#ifndef sgi
sprintf (ptydev, "/dev/ptyq%d", minor(fstat_buf.st_rdev));
! if ((*tty = enable_open (ttydev, O_RDWR)) < 0) {
close (*pty);
return(1);
}
***************
*** 1381,1387 ****
sprintf (ttydev, "/dev/ttyp%03d", devindex);
sprintf (ptydev, "/dev/pty/%03d", devindex);
! if ((*pty = open (ptydev, O_RDWR)) >= 0) {
/* We need to set things up for our next entry
* into this function!
*/
--- 1390,1396 ----
sprintf (ttydev, "/dev/ttyp%03d", devindex);
sprintf (ptydev, "/dev/pty/%03d", devindex);
! if ((*pty = enable_open (ptydev, O_RDWR)) >= 0) {
/* We need to set things up for our next entry
* into this function!
*/
***************
*** 1399,1405 ****
PTYCHAR2 [devindex];
/* for next time around loop or next entry to this function */
devindex++;
! if ((*pty = open (ptydev, O_RDWR)) >= 0) {
#ifdef sun
/* Need to check the process group of the pty.
* If it exists, then the slave pty is in use,
--- 1408,1414 ----
PTYCHAR2 [devindex];
/* for next time around loop or next entry to this function */
devindex++;
! if ((*pty = enable_open (ptydev, O_RDWR)) >= 0) {
#ifdef sun
/* Need to check the process group of the pty.
* If it exists, then the slave pty is in use,
***************
*** 1635,1641 ****
#endif /* LASTLOG */
#endif /* UTMP */
! screen->uid = getuid();
screen->gid = getgid();
#ifdef SIGTTOU
--- 1644,1650 ----
#endif /* LASTLOG */
#endif /* UTMP */
! screen->uid = Xterm_ruid;
screen->gid = getgid();
#ifdef SIGTTOU
***************
*** 2035,2040 ****
--- 2044,2050 ----
#endif /* USE_HANDSHAKE -- from near fork */
+ enable();
#ifdef USE_TTY_GROUP
{
#include <grp.h>
***************
*** 2058,2063 ****
--- 2068,2074 ----
/* change protection of tty */
chmod (ttydev, 0622);
#endif /* USE_TTY_GROUP */
+ disable();
/*
* set up the tty modes
***************
*** 2371,2377 ****
updwtmpx(WTMPX_FILE, &utmp);
#else
if (term->misc.login_shell &&
! (i = open(etc_wtmp, O_WRONLY|O_APPEND)) >= 0) {
write(i, (char *)&utmp, sizeof(struct utmp));
close(i);
}
--- 2382,2388 ----
updwtmpx(WTMPX_FILE, &utmp);
#else
if (term->misc.login_shell &&
! (i = enable_open(etc_wtmp, O_WRONLY|O_APPEND)) >= 0) {
write(i, (char *)&utmp, sizeof(struct utmp));
close(i);
}
***************
*** 2387,2394 ****
tslot = ttyslot();
added_utmp_entry = False;
{
if (pw && !resource.utmpInhibit &&
! (i = open(etc_utmp, O_WRONLY)) >= 0) {
bzero((char *)&utmp, sizeof(struct utmp));
(void) strncpy(utmp.ut_line,
ttydev + strlen("/dev/"),
--- 2398,2406 ----
tslot = ttyslot();
added_utmp_entry = False;
{
+ enable();
if (pw && !resource.utmpInhibit &&
! (i = enable_open(etc_utmp, O_WRONLY)) >= 0) {
bzero((char *)&utmp, sizeof(struct utmp));
(void) strncpy(utmp.ut_line,
ttydev + strlen("/dev/"),
***************
*** 2407,2413 ****
added_utmp_entry = True;
#ifdef WTMP
if (term->misc.login_shell &&
! (i = open(etc_wtmp, O_WRONLY|O_APPEND)) >= 0) {
int status;
status = write(i, (char *)&utmp,
sizeof(struct utmp));
--- 2419,2426 ----
added_utmp_entry = True;
#ifdef WTMP
if (term->misc.login_shell &&
! (i = enable_open(etc_wtmp, O_WRONLY|O_APPEND))
! >= 0) {
int status;
status = write(i, (char *)&utmp,
sizeof(struct utmp));
***************
*** 2416,2422 ****
#endif /* WTMP */
#ifdef LASTLOG
if (term->misc.login_shell &&
! (i = open(etc_lastlog, O_WRONLY)) >= 0) {
bzero((char *)&lastlog,
sizeof (struct lastlog));
(void) strncpy(lastlog.ll_line, ttydev +
--- 2429,2436 ----
#endif /* WTMP */
#ifdef LASTLOG
if (term->misc.login_shell &&
! (i = enable_open(etc_lastlog, O_WRONLY)) >= 0)
! {
bzero((char *)&lastlog,
sizeof (struct lastlog));
(void) strncpy(lastlog.ll_line, ttydev +
***************
*** 2435,2441 ****
#endif /* LASTLOG */
} else
tslot = -tslot;
! }
/* Let's pass our ttyslot to our parent so that it can
* clean up after us.
--- 2449,2455 ----
#endif /* LASTLOG */
} else
tslot = -tslot;
! }
/* Let's pass our ttyslot to our parent so that it can
* clean up after us.
***************
*** 2785,2791 ****
updwtmpx(WTMPX_FILE, &utmp);
#else
/* set wtmp entry if wtmp file exists */
! if ((fd = open(etc_wtmp, O_WRONLY | O_APPEND)) >= 0) {
i = write(fd, utptr, sizeof(utmp));
i = close(fd);
}
--- 2799,2806 ----
updwtmpx(WTMPX_FILE, &utmp);
#else
/* set wtmp entry if wtmp file exists */
! if ((fd = enable_open(etc_wtmp, O_WRONLY | O_APPEND)) >= 0)
! {
i = write(fd, utptr, sizeof(utmp));
i = close(fd);
}
***************
*** 2801,2807 ****
struct utmp utmp;
if (!resource.utmpInhibit && added_utmp_entry &&
! (!am_slave && tslot > 0 && (wfd = open(etc_utmp, O_WRONLY)) >= 0)){
bzero((char *)&utmp, sizeof(struct utmp));
lseek(wfd, (long)(tslot * sizeof(struct utmp)), 0);
write(wfd, (char *)&utmp, sizeof(struct utmp));
--- 2816,2823 ----
struct utmp utmp;
if (!resource.utmpInhibit && added_utmp_entry &&
! (!am_slave && tslot > 0 &&
! (wfd = enable_open(etc_utmp, O_WRONLY)) >= 0)) {
bzero((char *)&utmp, sizeof(struct utmp));
lseek(wfd, (long)(tslot * sizeof(struct utmp)), 0);
write(wfd, (char *)&utmp, sizeof(struct utmp));
***************
*** 2808,2814 ****
close(wfd);
#ifdef WTMP
if (term->misc.login_shell &&
! (wfd = open(etc_wtmp, O_WRONLY | O_APPEND)) >= 0) {
(void) strncpy(utmp.ut_line, ttydev +
sizeof("/dev"), sizeof (utmp.ut_line));
time(&utmp.ut_time);
--- 2824,2830 ----
close(wfd);
#ifdef WTMP
if (term->misc.login_shell &&
! (wfd = enable_open(etc_wtmp, O_WRONLY | O_APPEND)) >= 0) {
(void) strncpy(utmp.ut_line, ttydev +
sizeof("/dev"), sizeof (utmp.ut_line));
time(&utmp.ut_time);
***************
*** 2827,2832 ****
--- 2843,2849 ----
if (!am_slave) {
/* restore ownership of tty and pty */
+ enable();
chown (ttydev, 0, 0);
#ifndef sgi
chown (ptydev, 0, 0);
***************
*** 2837,2842 ****
--- 2854,2860 ----
#ifndef sgi
chmod (ptydev, 0666);
#endif /* !sgi */
+ disable();
}
exit(n);
SIGNAL_RETURN;
***************
*** 3062,3065 ****
--- 3080,3132 ----
return killpg (pid, sig);
#endif
#endif
+ }
+
+ disable()
+ {
+ #ifdef SAFESETUID
+ #ifdef _POSIX_SAVED_IDS
+ seteuid (Xterm_ruid);
+ #else
+ setreuid (Xterm_euid, Xterm_ruid);
+ #endif /* _POSIX_SAVED_IDS */
+
+ #ifdef DEBUG2
+ fprintf (stderr, "Disabled: uid=%6d, euid=%6d, pid=%6d\n",
+ getuid(), geteuid(), getpid());
+ #endif /* DEBUG2 */
+ #endif /* SAFESETUID */
+ }
+
+ enable()
+ {
+ #ifdef SAFESETUID
+ #ifdef _POSIX_SAVED_IDS
+ seteuid (Xterm_euid);
+ #else
+ setreuid (Xterm_ruid, Xterm_euid);
+ #endif /* _POSIX_SAVED_IDS */
+
+ #ifdef DEBUG2
+ fprintf (stderr, " Enabled: uid=%6d, euid=%6d, pid=%6d\n",
+ getuid(), geteuid(), getpid());
+ #endif /* DEBUG2 */
+ #endif /* SAFESETUID */
+ }
+
+ enable_open(path, flags, mode)
+ char *path;
+ int flags;
+ mode_t mode;
+ {
+ int ret;
+
+ #ifdef DEBUG2
+ fprintf(stderr, "opening %s\n", path);
+ #endif /* DEBUG2 */
+
+ enable();
+ ret = open(path, flags, mode);
+ disable();
+ return ret;
}
*** /tmp/,RCSt1019396 Tue Mar 14 12:49:24 1995
--- misc.c Tue Mar 14 12:42:20 1995
***************
*** 526,531 ****
--- 526,537 ----
#endif /* SYSV */
#endif /* ALLOWLOGFILEEXEC */
+
+ #ifdef DEBUG2
+ fprintf (stderr, " Logging: uid=%6d, euid=%6d, pid=%6d\n",
+ getuid(), geteuid(), getpid());
+ #endif /* DEBUG2 */
+
if(screen->logging || (screen->inhibit & I_LOG))
return;
if(screen->logfile == NULL || *screen->logfile == 0) {
