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