Based upon the discussion of xterm a couple of days ago, I have been
working on a couple changes to reduce the privs of xterm in general,
by reducing the scope of the utmp egid by opening utmp early, improving
the unveil calls to match, and then tightening the pledge.

Additionally, some file-related functions not used by our xterm because
of feature disabling, are become hidden behind #ifdef, and I update the
manual page.

It's a jumbo diff, for testing in snaps, to see if there is any fallout.
I tried to tighten a bunch of other really ugly things I found (nested
select and poll calls, oh boy, with short-cut exit paths to workaround
the introduced problems).  But, for now, this is how far I think we can
go in first few steps.

As I said, this is in snaps.

Index: button.c
===================================================================
RCS file: /cvs/xenocara/app/xterm/button.c,v
retrieving revision 1.43
diff -u -r1.43 button.c
--- button.c    25 Apr 2022 19:20:37 -0000      1.43
+++ button.c    16 May 2022 04:54:55 -0000
@@ -5935,6 +5935,7 @@
     return result;
 }
 
+#if OPT_EXEC_XTERM
 /* execute the command after forking.  The main process frees its data */
 static void
 executeCommand(pid_t pid, char **argv)
@@ -6038,6 +6039,7 @@
        }
     }
 }
+#endif /* OPT_EXEC_XTERM */
 
 static void
 reallyInsertFormatted(Widget w, char *format, char *data, CELL *start, CELL 
*finish)
Index: charproc.c
===================================================================
RCS file: /cvs/xenocara/app/xterm/charproc.c,v
retrieving revision 1.51
diff -u -r1.51 charproc.c
--- charproc.c  25 Apr 2022 19:20:37 -0000      1.51
+++ charproc.c  16 May 2022 04:53:31 -0000
@@ -384,8 +384,10 @@
     { "scroll-lock",           HandleScrollLock },
 #endif
 #if OPT_SELECTION_OPS
+#if OPT_EXEC_XTERM
     { "exec-formatted",                HandleExecFormatted },
     { "exec-selectable",       HandleExecSelectable },
+#endif /* OPT_EXEC_XTERM */
     { "insert-formatted",      HandleInsertFormatted },
     { "insert-selectable",     HandleInsertSelectable },
 #endif
Index: main.c
===================================================================
RCS file: /cvs/xenocara/app/xterm/main.c,v
retrieving revision 1.53
diff -u -r1.53 main.c
--- main.c      25 Apr 2022 19:20:37 -0000      1.53
+++ main.c      17 May 2022 04:18:19 -0000
@@ -306,7 +306,7 @@
 #include <sys/param.h>         /* for NOFILE */
 #endif
 
-#if defined(BSD) && (BSD >= 199103)
+#if defined(BSD) && (BSD >= 199103) && !defined(__OpenBSD__)
 #define WTMP
 #endif
 
@@ -397,6 +397,7 @@
 #define UTMP_FILENAME UTMP_FILE
 #elif defined(_PATH_UTMP)
 #define UTMP_FILENAME _PATH_UTMP
+int utmp_fd = -1;
 #else
 #define UTMP_FILENAME "/etc/utmp"
 #endif
@@ -2784,6 +2785,13 @@
 
     spawnXTerm(term, line_speed);
 
+    /* xterm (parent) grabs a fd for the utmp file now, while it
+     * has egid = "utmp".  Then discard the egid. */
+    setEffectiveGroup(save_egid);
+    utmp_fd = open(etc_utmp, O_WRONLY | O_CLOEXEC);
+    setresgid(save_rgid, save_rgid, save_rgid);
+    save_egid = -1;
+
 #ifndef VMS
     /* Child process is out there, let's catch its termination */
 
@@ -2903,6 +2911,7 @@
 #endif
 
     {
+#if OPT_EXEC_XTERM
         String data = NULL;
         getKeymapResources(SHELL_OF(term), "vt100", "VT100", XtRString, &data, 
sizeof(data));
         if (data &&
@@ -2912,50 +2921,59 @@
                 xtermWarning("pledge\n");
                 exit(1);
             }
-        } else {
+        } else
+#endif /* OPT_EXEC_XTERM */
+        {
             char *env;
+
             if ((env = getenv("HOME"))) {
                 char homefile[PATH_MAX];
                /* we ignore  unveil() errors - they are no relevant here */
-                snprintf(homefile, sizeof homefile, "%s/.fonts", env);
-                unveil(homefile, "r");
-                snprintf(homefile, sizeof homefile, "%s/.cache/fontconfig",
-                         env);
-                unveil(homefile, "r");
-                snprintf(homefile, sizeof homefile, "%s/.icons", env);
-                unveil(homefile, "r");
+                if (snprintf(homefile, sizeof homefile, "%s/.fonts",
+                   env) <= sizeof(homefile))
+                       unveil(homefile, "r");
+                if (snprintf(homefile, sizeof homefile, "%s/.cache/fontconfig",
+                    env) <= sizeof(homefile))
+                       unveil(homefile, "r");
+                if (snprintf(homefile, sizeof homefile, "%s/.icons",
+                    env) <= sizeof(homefile))
+                       unveil(homefile, "r");
             }
             if ((env = getenv("XDG_CONFIG_HOME"))) {
                 char xdgfile[PATH_MAX];
 
-                snprintf(xdgfile, sizeof xdgfile, "%s/fontconfig", env);
-                unveil(xdgfile, "r");
-                snprintf(xdgfile, sizeof xdgfile, "%s/icons", env);
-                unveil(xdgfile, "r");
+                if (snprintf(xdgfile, sizeof xdgfile, "%s/fontconfig",
+                   env) <= sizeof(xdgfile))
+                       unveil(xdgfile, "r");
+                if (snprintf(xdgfile, sizeof xdgfile, "%s/icons",
+                   env) <= sizeof(xdgfile))
+                       unveil(xdgfile, "r");
             }
             if ((env = getenv("XDG_DATA_HOME"))) {
                 char xdgfile[PATH_MAX];
 
-                snprintf(xdgfile, sizeof xdgfile, "%s/fontconfig", env);
-                unveil(xdgfile, "r");
-                snprintf(xdgfile, sizeof xdgfile, "%s/icons", env);
-                unveil(xdgfile, "r");
+                if (snprintf(xdgfile, sizeof xdgfile, "%s/fontconfig",
+                   env) <= sizeof(xdgfile))
+                       unveil(xdgfile, "r");
+                if (snprintf(xdgfile, sizeof xdgfile, "%s/icons",
+                   env) <= sizeof(xdgfile))
+                       unveil(xdgfile, "r");
             }
             if ((env = getenv("XDG_CACHE_HOME"))) {
                 char xdgfile[PATH_MAX];
 
-                snprintf(xdgfile, sizeof xdgfile, "%s/fontconfig", env);
-                unveil(xdgfile, "r"); 
+                if (snprintf(xdgfile, sizeof xdgfile, "%s/fontconfig",
+                   env) <= sizeof(xdgfile))
+                       unveil(xdgfile, "r"); 
            }
+
            unveil("/usr/X11R6", "r");
             unveil("/usr/local/share/fonts", "r");
            unveil("/var/cache/fontconfig", "r");
            unveil("/usr/local/share/icons", "r");
            unveil("/usr/local/lib/X11/icons", "r");
-            unveil(etc_utmp, "w");
-           unveil(etc_wtmp, "w");
 
-            if (pledge("stdio rpath wpath id proc tty", NULL) == -1) {
+            if (pledge("stdio rpath proc tty", NULL) == -1) {
                xtermWarning("pledge\n");
                exit(1);
             }
@@ -5483,10 +5501,12 @@
     if (!resource.utmpInhibit && added_utmp_entry &&
        (am_slave < 0 && tslot > 0)) {
 #if defined(USE_UTMP_SETGID)
-       setEffectiveGroup(save_egid);
-       TRACE_IDS;
+       if (save_egid != -1) {
+               setEffectiveGroup(save_egid);
+               TRACE_IDS;
+       }
 #endif
-       if ((wfd = open(etc_utmp, O_WRONLY)) >= 0) {
+       if ((wfd = utmp_fd) != -1 || (wfd = open(etc_utmp, O_WRONLY)) >= 0) {
            memset(&utmp, 0, sizeof(utmp));
            lseek(wfd, (long) (tslot * sizeof(utmp)), 0);
            IGNORE_RC(write(wfd, (char *) &utmp, sizeof(utmp)));
@@ -5504,8 +5524,10 @@
        }
 #endif /* WTMP */
 #ifdef USE_UTMP_SETGID
-       disableSetGid();
-       TRACE_IDS;
+       if (save_egid != -1) {
+               disableSetGid();
+               TRACE_IDS;
+       }
 #endif
     }
 #endif /* USE_SYSV_UTMP */
Index: misc.c
===================================================================
RCS file: /cvs/xenocara/app/xterm/misc.c,v
retrieving revision 1.45
diff -u -r1.45 misc.c
--- misc.c      25 Apr 2022 19:20:37 -0000      1.45
+++ misc.c      16 May 2022 23:53:01 -0000
@@ -2140,6 +2140,7 @@
            tstruct->tm_sec);
 }
 
+#if OPT_SCREEN_DUMPS
 FILE *
 create_printfile(XtermWidget xw, const char *suffix)
 {
@@ -2171,7 +2172,9 @@
     fp = (fd >= 0) ? fdopen(fd, "wb") : NULL;
     return fp;
 }
+#endif /* OPT_SCREEN_DUMPS */
 
+#if OPT_SCREEN_DUMPS || defined(ALLOWLOGGING)
 int
 open_userfile(uid_t uid, gid_t gid, char *path, Bool append)
 {
@@ -2319,6 +2322,7 @@
     }
 }
 #endif /* !VMS */
+#endif /* OPT_SCREEN_DUMPS || defined(ALLOWLOGGING) */
 
 int
 xtermResetIds(TScreen *screen)
Index: xterm.man
===================================================================
RCS file: /cvs/xenocara/app/xterm/xterm.man,v
retrieving revision 1.57
diff -u -r1.57 xterm.man
--- xterm.man   25 Apr 2022 19:20:38 -0000      1.57
+++ xterm.man   16 May 2022 03:45:41 -0000
@@ -7255,84 +7255,11 @@
 Invokes the \fBSVG Screen Dump\fP feature.
 .TP 8
 .B "exec\-formatted(\fIformat\fP, \fIsourcename\fP [, \&...\&])"
-Execute an external command,
-using the current selection for part of the command's parameters.
-The first parameter, \fIformat\fP gives the basic command.
-Succeeding parameters specify the selection source
-as in \fBinsert\-selection\fP.
-.IP
-The \fIformat\fP parameter allows these substitutions:
-.RS
-.TP 5
-%%
-inserts a "%".
-.TP 5
-%P
-the screen-position at the beginning of the highlighted region,
-as a semicolon-separated pair of integers using the
-values that the CUP control sequence would use.
-.TP 5
-%p
-the screen-position after the beginning of the highlighted region,
-using the same convention as \*(``%P\*(''.
-.TP 5
-%S
-the length of the string that \*(``%s\*('' would insert.
-.TP 5
-%s
-the content of the selection, unmodified.
-.TP 5
-%T
-the length of the string that \*(``%t\*('' would insert.
-.TP 5
-%t
-the selection, trimmed of leading/trailing whitespace.
-Embedded spaces (and newlines) are copied as is.
-.TP 5
-%R
-the length of the string that \*(``%r\*('' would insert.
-.TP 5
-%r
-the selection, trimmed of trailing whitespace.
-.TP 5
-%V
-the video attributes at the beginning of the highlighted region,
-as a semicolon-separated list of integers using the
-values that the SGR control sequence would use.
-.TP 5
-%v
-the video attributes after the end of the highlighted region,
-using the same convention as \*(``%V\*(''.
-.RE
-.IP
-After constructing the command-string,
-\fI\*n\fP forks a subprocess and executes the command,
-which completes independently of \fI\*n\fP.
-.IP
-For example, this translation would invoke a new \fI\*n\fP process
-to view a file whose name is selected while holding the shift key down.
-The new process is started when the mouse button is released:
-.NS
-*VT100*translations: #override Shift \\\&
-    <Btn1Up>:\fBexec\-formatted\fP("xterm \-e view \*(AQ%t\*(AQ", \fBSELECT\fP)
+This feature is disabled.
 .NE
 .TP 8
 .B "exec\-selectable(\fIformat\fP, \fIonClicks\fP)"
-Execute an external command,
-using data copied from the screen for part of the command's parameters.
-The first parameter, \fIformat\fP gives
-the basic command as in \fBexec\-formatted\fP.
-The second parameter specifies the method for copying
-the data as in the \fBon2Clicks\fP resource.
-.TP 8
-.B "fullscreen(\fIon/off/toggle\fP)"
-This action sets, unsets or toggles the \fBfullscreen\fP resource.
-.TP 8
-.B "hard\-reset()"
-This action resets the scrolling region, tabs, window size, and cursor keys
-and clears the screen.
-It is also invoked from the \fBhardreset\fP
-entry in \fIvtMenu\fP.
+This feature is disabled.
 .TP 8
 .B "iconify()"
 Iconifies the window.
@@ -7881,19 +7808,7 @@
 The effect is identical to a soft reset (DECSTR) control sequence.
 .TP 8
 .B "spawn\-new\-terminal(\fIparams\fP)"
-Spawn a new \fI\*n\fP process.
-This is available on systems which have a modern version of the
-process filesystem, e.g., \*(``/proc\*('', which \fI\*n\fP can read.
-.IP
-Use the \*(``cwd\*('' process entry, e.g., /proc/12345/cwd to obtain the
-working directory of the process which is running in the current \fI\*n\fP.
-.IP
-On systems which have the \*(``exe\*('' process entry, e.g., /proc/12345/exe,
-use this to obtain the actual executable.
-Otherwise, use the \fB$PATH\fP variable to find \fI\*n\fP.
-.IP
-If parameters are given in the action,
-pass them to the new \fI\*n\fP process.
+This feature is disabled.
 .TP 8
 .B "start\-cursor\-extend()"
 This action is similar to \fBselect\-extend\fP except that the

Reply via email to