CVSROOT:        /cvs
Module name:    xenocara
Changes by:     dera...@cvs.openbsd.org 2022/05/22 08:43:01

Modified files:
        app/xterm      : main.c 

Log message:
Back in darker times, xterm was setuid root to add a user entry to the
utmp and wtmp files at startup. real-uid / effective-uid flipping was
used to cleanup the records in these two files at logout-time.  Over
time it was recognized that setuid root is too dangerous, later on
even uid flipping became an unacceptable practice (because an attacker
who finds a bug will simply flip the uid back to root before
continuing exploitation).  Some OS's helped xterm (and other similar
login-related tooling) by making utmp writeable by a new utmp group,
but most did not do the same for wtmp.  xterm started using this new
utmp gid, and the wtmp code moved to "try, and if it fails, ignore the
failure".

The obvious way to use this uid is for xterm to open the utmp file for
write (early on), discard the egid, and keep the file descriptor
around until utmp cleanup at termination.  10-20 lines of code, maybe.
But no,.... that's not what happened.

The previous setuid root flipping code (which is nearly a hundred
lines of #ifdef-wrapped code for portability reasons) was copied and
repurposed by adding new #ifdef code for setgid utmp flipping, and
thus nearly a hundred lines of #ifdef-wrapped code was added).  setgid
flipping has less severe security risks than setuid flipping, but it
is remains an excessively strong and unneccessary power (compared to a
single writeable fd).

When pledge() arrived on the scene, "wpath" was required so that the
utmp file could be opened late, and "id" was required to support egid
flipping.  unveil() arrived on the scene, and the utmp path was added
to the list of viable paths, once again not considering that an incorrect
approach was being taken by the code.

I tried rewriting the portable USE_UTMP_SETGID code to follow the
open-drop-reuse-fd approach, to help out upstream xterm, but it is such
a brain-melting shitshow I gave up, we'll have a (small) intrusive patch
which opens utmp early, drops the gid, and reuses the fd later on.

Maybe upstream will take care of this eventually to reduce the risk of
egid other operating systems.
ok matthieu, much feedback from millert

Reply via email to