Delete support for the LESSOPEN and LESSCLOSE environment variables
authorguenther <guenther@openbsd.org>
Sun, 14 Apr 2024 18:11:54 +0000 (18:11 +0000)
committerguenther <guenther@openbsd.org>
Sun, 14 Apr 2024 18:11:54 +0000 (18:11 +0000)
aka the "Input Preprocessor": it's been a source for multiple
security bugs in the past as everything has to deal with handling
arbitrary filenames and generally resulted in multiple TOCTOU issues.
The base system never included a default LESSOPEN setting like some
Linux distributions did, but it's a suds-filled sink full of knives
to try to use safely.

ok tb@ deraadt@ millert@

usr.bin/less/ch.c
usr.bin/less/command.c
usr.bin/less/edit.c
usr.bin/less/filename.c
usr.bin/less/funcs.h
usr.bin/less/less.1
usr.bin/less/less.h
usr.bin/less/less.hlp
usr.bin/less/main.c
usr.bin/less/opttbl.c

index d7c0aa3..8ca21ea 100644 (file)
@@ -778,7 +778,7 @@ ch_close(void)
        if (thisfile == NULL)
                return;
 
-       if (ch_flags & (CH_CANSEEK|CH_POPENED|CH_HELPFILE)) {
+       if (ch_flags & (CH_CANSEEK|CH_HELPFILE)) {
                /*
                 * We can seek or re-open, so we don't need to keep buffers.
                 */
@@ -790,11 +790,8 @@ ch_close(void)
                /*
                 * We don't need to keep the file descriptor open
                 * (because we can re-open it.)
-                * But don't really close it if it was opened via popen(),
-                * because pclose() wants to close it.
                 */
-               if (!(ch_flags & CH_POPENED))
-                       close(ch_file);
+               close(ch_file);
                ch_file = -1;
        } else {
                keepstate = TRUE;
index 4fc19d9..d9464a3 100644 (file)
@@ -35,7 +35,6 @@ extern int hshift;
 extern int show_attn;
 extern off_t highest_hilite;
 extern char *every_first_cmd;
-extern char *curr_altfilename;
 extern char version[];
 extern struct scrpos initial_scrpos;
 extern IFILE curr_ifile;
@@ -1412,10 +1411,6 @@ again:
                                error("Cannot edit standard input", NULL);
                                break;
                        }
-                       if (curr_altfilename != NULL) {
-                               error("WARNING: This file was viewed via "
-                                   "LESSOPEN", NULL);
-                       }
                        /*
                         * Expand the editor prototype string
                         * and pass it to the system to execute.
index eff4d28..15a5be6 100644 (file)
@@ -35,9 +35,6 @@ extern char *namelogfile;
 dev_t curr_dev;
 ino_t curr_ino;
 
-char *curr_altfilename = NULL;
-static void *curr_altpipe;
-
 
 /*
  * Textlist functions deal with a list of words separated by spaces.
@@ -146,16 +143,6 @@ close_file(void)
         * Close the file descriptor, unless it is a pipe.
         */
        ch_close();
-       /*
-        * If we opened a file using an alternate name,
-        * do special stuff to close it.
-        */
-       if (curr_altfilename != NULL) {
-               close_altfile(curr_altfilename, get_filename(curr_ifile),
-                   curr_altpipe);
-               free(curr_altfilename);
-               curr_altfilename = NULL;
-       }
        curr_ifile = NULL;
        curr_ino = curr_dev = 0;
 }
@@ -185,10 +172,7 @@ edit_ifile(IFILE ifile)
        int no_display;
        int chflags;
        char *filename;
-       char *open_filename;
        char *qopen_filename;
-       char *alt_filename;
-       void *alt_pipe;
        IFILE was_curr_ifile;
        PARG parg;
 
@@ -200,11 +184,9 @@ edit_ifile(IFILE ifile)
        }
 
        /*
-        * We must close the currently open file now.
-        * This is necessary to make the open_altfile/close_altfile pairs
-        * nest properly (or rather to avoid nesting at all).
-        * {{ Some stupid implementations of popen() mess up if you do:
-        *    fA = popen("A"); fB = popen("B"); pclose(fA); pclose(fB); }}
+        * We close the currently open file now.  This was done before
+        * to avoid linked popen/pclose pairs from LESSOPEN, but there
+        * may other code that has come to rely on this restriction.
         */
        end_logfile();
        was_curr_ifile = save_curr_ifile();
@@ -233,47 +215,28 @@ edit_ifile(IFILE ifile)
        }
 
        filename = estrdup(get_filename(ifile));
-       /*
-        * See if LESSOPEN specifies an "alternate" file to open.
-        */
-       alt_pipe = NULL;
-       alt_filename = open_altfile(filename, &f, &alt_pipe);
-       open_filename = (alt_filename != NULL) ? alt_filename : filename;
-       qopen_filename = shell_unquote(open_filename);
+       qopen_filename = shell_unquote(filename);
 
        chflags = 0;
-       if (strcmp(open_filename, helpfile()) == 0)
+       if (strcmp(filename, helpfile()) == 0)
                chflags |= CH_HELPFILE;
-       if (alt_pipe != NULL) {
-               /*
-                * The alternate "file" is actually a pipe.
-                * f has already been set to the file descriptor of the pipe
-                * in the call to open_altfile above.
-                * Keep the file descriptor open because it was opened
-                * via popen(), and pclose() wants to close it.
-                */
-               chflags |= CH_POPENED;
-       } else if (strcmp(open_filename, "-") == 0) {
+       if (strcmp(filename, "-") == 0) {
                /*
                 * Use standard input.
                 * Keep the file descriptor open because we can't reopen it.
                 */
                f = fd0;
                chflags |= CH_KEEPOPEN;
-       } else if (strcmp(open_filename, FAKE_EMPTYFILE) == 0) {
+       } else if (strcmp(filename, FAKE_EMPTYFILE) == 0) {
                f = -1;
                chflags |= CH_NODATA;
-       } else if ((parg.p_string = bad_file(open_filename)) != NULL) {
+       } else if ((parg.p_string = bad_file(filename)) != NULL) {
                /*
                 * It looks like a bad file.  Don't try to open it.
                 */
                error("%s", &parg);
                free(parg.p_string);
 err1:
-               if (alt_filename != NULL) {
-                       close_altfile(alt_filename, filename, alt_pipe);
-                       free(alt_filename);
-               }
                del_ifile(ifile);
                free(qopen_filename);
                free(filename);
@@ -323,8 +286,6 @@ err1:
                unsave_ifile(was_curr_ifile);
        }
        curr_ifile = ifile;
-       curr_altfilename = alt_filename;
-       curr_altpipe = alt_pipe;
        set_open(curr_ifile); /* File has been opened */
        get_pos(curr_ifile, &initial_scrpos);
        new_file = TRUE;
index dfe998f..0aaecac 100644 (file)
@@ -26,7 +26,6 @@
 
 extern int force_open;
 extern int secure;
-extern int use_lessopen;
 extern int ctldisp;
 extern int utf_mode;
 extern IFILE curr_ifile;
@@ -518,182 +517,6 @@ lglob(char *filename)
        return (gfilename);
 }
 
-/*
- * Expand LESSOPEN or LESSCLOSE.  Returns a newly allocated string
- * on success, NULL otherwise.
- */
-static char *
-expand_pct_s(const char *fmt, ...)
-{
-       int             n;
-       int             len;
-       char            *r, *d;
-       const char      *f[3];          /* max expansions + 1 for NULL */
-       va_list         ap;
-
-       va_start(ap, fmt);
-       for (n = 0; n < ((sizeof (f)/sizeof (f[0])) - 1); n++) {
-               f[n] = (const char *)va_arg(ap, const char *);
-               if (f[n] == NULL) {
-                       break;
-               }
-       }
-       va_end(ap);
-       f[n] = NULL;    /* terminate list */
-
-       len = strlen(fmt) + 1;
-       for (n = 0; f[n] != NULL; n++) {
-               len += strlen(f[n]);    /* technically could -2 for "%s" */
-       }
-       r = ecalloc(len, sizeof (char));
-
-       for (n = 0, d = r; *fmt != 0; ) {
-               if (*fmt != '%') {
-                       *d++ = *fmt++;
-                       continue;
-               }
-               fmt++;
-               /* Permit embedded "%%" */
-               switch (*fmt) {
-               case '%':
-                       *d++ = '%';
-                       fmt++;
-                       break;
-               case 's':
-                       if (f[n] == NULL) {
-                               va_end(ap);
-                               free(r);
-                               return (NULL);
-                       }
-                       (void) strlcpy(d, f[n++], r + len - d);
-                       fmt++;
-                       d += strlen(d);
-                       break;
-               default:
-                       va_end(ap);
-                       free(r);
-                       return (NULL);
-               }
-       }
-       *d = '\0';
-       return (r);
-}
-
-/*
- * See if we should open a "replacement file"
- * instead of the file we're about to open.
- */
-char *
-open_altfile(char *filename, int *pf, void **pfd)
-{
-       char *lessopen;
-       char *cmd;
-       FILE *fd;
-       int returnfd = 0;
-
-       if (!use_lessopen || secure)
-               return (NULL);
-       ch_ungetchar(-1);
-       if ((lessopen = lgetenv("LESSOPEN")) == NULL)
-               return (NULL);
-       while (*lessopen == '|') {
-               /*
-                * If LESSOPEN starts with a |, it indicates
-                * a "pipe preprocessor".
-                */
-               lessopen++;
-               returnfd++;
-       }
-       if (*lessopen == '-') {
-               /*
-                * Lessopen preprocessor will accept "-" as a filename.
-                */
-               lessopen++;
-       } else {
-               if (strcmp(filename, "-") == 0)
-                       return (NULL);
-       }
-
-       if ((cmd = expand_pct_s(lessopen, filename, NULL)) == NULL) {
-               error("Invalid LESSOPEN variable", NULL);
-               return (NULL);
-       }
-       fd = shellcmd(cmd);
-       free(cmd);
-       if (fd == NULL) {
-               /*
-                * Cannot create the pipe.
-                */
-               return (NULL);
-       }
-       if (returnfd) {
-               int f;
-               char c;
-
-               /*
-                * Read one char to see if the pipe will produce any data.
-                * If it does, push the char back on the pipe.
-                */
-               f = fileno(fd);
-               if (read(f, &c, 1) != 1) {
-                       /*
-                        * Pipe is empty.
-                        * If more than 1 pipe char was specified,
-                        * the exit status tells whether the file itself
-                        * is empty, or if there is no alt file.
-                        * If only one pipe char, just assume no alt file.
-                        */
-                       int status = pclose(fd);
-                       if (returnfd > 1 && status == 0) {
-                               *pfd = NULL;
-                               *pf = -1;
-                               return (estrdup(FAKE_EMPTYFILE));
-                       }
-                       return (NULL);
-               }
-               ch_ungetchar(c);
-               *pfd = (void *) fd;
-               *pf = f;
-               return (estrdup("-"));
-       }
-       cmd = readfd(fd);
-       pclose(fd);
-       if (*cmd == '\0')
-               /*
-                * Pipe is empty.  This means there is no alt file.
-                */
-               return (NULL);
-       return (cmd);
-}
-
-/*
- * Close a replacement file.
- */
-void
-close_altfile(char *altfilename, char *filename, void *pipefd)
-{
-       char *lessclose;
-       FILE *fd;
-       char *cmd;
-
-       if (secure)
-               return;
-       if (pipefd != NULL) {
-               pclose((FILE *)pipefd);
-       }
-       if ((lessclose = lgetenv("LESSCLOSE")) == NULL)
-               return;
-       cmd = expand_pct_s(lessclose, filename, altfilename, NULL);
-       if (cmd == NULL) {
-               error("Invalid LESSCLOSE variable", NULL);
-               return;
-       }
-       fd = shellcmd(cmd);
-       free(cmd);
-       if (fd != NULL)
-               (void) pclose(fd);
-}
-
 /*
  * Is the specified file a directory?
  */
index 2e707c3..ec86a31 100644 (file)
@@ -122,8 +122,6 @@ char *fexpand(char *);
 char *fcomplete(char *);
 int bin_file(int f);
 char *lglob(char *);
-char *open_altfile(char *, int *, void **);
-void close_altfile(char *, char *, void *);
 int is_dir(char *);
 char *bad_file(char *);
 off_t filesize(int);
index 10811ed..fca7496 100644 (file)
@@ -1,4 +1,4 @@
-.\"    $OpenBSD: less.1,v 1.59 2021/12/10 17:26:54 schwarze Exp $
+.\"    $OpenBSD: less.1,v 1.60 2024/04/14 18:11:54 guenther Exp $
 .\"
 .\" Copyright (C) 1984-2012  Mark Nudelman
 .\"
@@ -23,7 +23,7 @@
 .\" OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN
 .\" IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 .\"
-.Dd $Mdocdate: December 10 2021 $
+.Dd $Mdocdate: April 14 2024 $
 .Dt LESS 1
 .Os
 .Sh NAME
@@ -304,16 +304,6 @@ environment variable is set, or if a lesskey file is found in a standard place
 (see
 .Sx KEY BINDINGS ) ,
 it is also used as a lesskey file.
-.It Fl L | -no-lessopen
-Ignore the
-.Ev LESSOPEN
-environment variable (see the
-.Sx INPUT PREPROCESSOR
-section below).
-This option can be set from within
-.Nm less ,
-but it will apply only to files opened subsequently, not to the
-file which is currently open.
 .It Fl M | -LONG-PROMPT
 Causes
 .Nm
@@ -1258,160 +1248,6 @@ On
 .Ox ,
 the system-wide lesskey file is
 .Pa /etc/sysless .
-.Sh INPUT PREPROCESSOR
-You may define an "input preprocessor" for
-.Nm less .
-Before
-.Nm less
-opens a file, it first gives your input preprocessor a chance to modify the
-way the contents of the file are displayed.
-An input preprocessor is simply an executable program (or shell script),
-which writes the contents of the file to a different file,
-called the replacement file.
-The contents of the replacement file are then displayed
-in place of the contents of the original file.
-However, it will appear to the user as if the original file is opened;
-that is,
-.Nm less
-will display the original filename as the name of the current file.
-.Pp
-An input preprocessor receives one command line argument, the original filename,
-as entered by the user.
-It should create the replacement file, and when finished
-print the name of the replacement file to its standard output.
-If the input preprocessor does not output a replacement filename,
-.Nm
-uses the original file, as normal.
-The input preprocessor is not called when viewing standard input.
-To set up an input preprocessor, set the
-.Ev LESSOPEN
-environment variable to a command line which will invoke your
-input preprocessor.
-This command line should include one occurrence of the string "%s",
-which will be replaced by the filename
-when the input preprocessor command is invoked.
-.Pp
-When
-.Nm
-closes a file opened in such a way, it will call another program,
-called the input postprocessor,
-which may perform any desired clean-up action (such as deleting the
-replacement file created by
-.Ev LESSOPEN ) .
-This program receives two command line arguments, the original filename
-as entered by the user, and the name of the replacement file.
-To set up an input postprocessor, set the
-.Ev LESSCLOSE
-environment variable to a command line which will invoke your
-input postprocessor.
-It may include two occurrences of the string "%s";
-the first is replaced with the original name of the file and the second
-with the name of the replacement file, which was output by
-.Ev LESSOPEN .
-.Pp
-For example, these two scripts will allow you
-to keep files in compressed format, but still let
-.Nm
-view them directly:
-.Pp
-lessopen.sh:
-.Bd -literal -offset indent
-#! /bin/sh
-case "$1" in
-*.Z)   uncompress -c $1  >/tmp/less.$$  2>/dev/null
-       if [ -s /tmp/less.$$ ]; then
-               echo /tmp/less.$$
-       else
-               rm -f /tmp/less.$$
-       fi
-       ;;
-esac
-.Ed
-.Pp
-lessclose.sh:
-.Bd -literal -offset indent
-#! /bin/sh
-rm $2
-.Ed
-.Pp
-To use these scripts, put them both where they can be executed and
-set LESSOPEN="lessopen.sh\ %s", and LESSCLOSE="lessclose.sh\ %s\ %s".
-More complex LESSOPEN and LESSCLOSE scripts may be written
-to accept other types of compressed files, and so on.
-.Pp
-It is also possible to set up an input preprocessor to
-pipe the file data directly to
-.Nm less ,
-rather than putting the data into a replacement file.
-This avoids the need to decompress the entire file before starting to view it.
-An input preprocessor that works this way is called an input pipe.
-An input pipe, instead of writing the name of a replacement file on
-its standard output,
-writes the entire contents of the replacement file on its standard output.
-If the input pipe does not write any characters on its standard output,
-then there is no replacement file and
-.Nm
-uses the original file, as normal.
-To use an input pipe, make the first character in the
-.Ev LESSOPEN
-environment variable a vertical bar (|) to signify that the
-input preprocessor is an input pipe.
-.Pp
-For example, this script will work like the previous example scripts:
-.Pp
-lesspipe.sh:
-.Bd -literal -offset indent
-#! /bin/sh
-case "$1" in
-*.Z)   uncompress -c $1  2>/dev/null
-*)     exit 1
-       ;;
-esac
-exit $?
-.Ed
-.Pp
-To use this script, put it where it can be executed and set
-LESSOPEN="|lesspipe.sh %s".
-.Pp
-Note that a preprocessor cannot output an empty file, since that
-is interpreted as meaning there is no replacement, and
-the original file is used.
-To avoid this, if
-.Ev LESSOPEN
-starts with two vertical bars,
-the exit status of the script becomes meaningful.
-If the exit status is zero, the output is considered to be
-replacement text, even if it empty.
-If the exit status is nonzero, any output is ignored and the
-original file is used.
-For compatibility with previous versions of
-.Nm less ,
-if
-.Ev LESSOPEN
-starts with only one vertical bar, the exit status
-of the preprocessor is ignored.
-.Pp
-When an input pipe is used, a LESSCLOSE postprocessor can be used,
-but it is usually not necessary since there is no replacement file to clean up.
-In this case, the replacement file name passed to the LESSCLOSE
-postprocessor is "-".
-.Pp
-For compatibility with previous versions of
-.Nm less ,
-the input preprocessor or pipe is not used if
-.Nm
-is viewing standard input.
-However, if the first character of LESSOPEN is a dash (-),
-the input preprocessor is used on standard input as well as other files.
-In this case, the dash is not considered to be part of
-the preprocessor command.
-If standard input is being viewed, the input preprocessor is passed
-a file name consisting of a single dash.
-Similarly, if the first two characters of LESSOPEN are vertical bar and dash
-(|-) or two vertical bars and a dash (||-),
-the input pipe is used on standard input as well as other files.
-Again, in this case the dash is not considered to be part of
-the input pipe command.
 .Sh NATIONAL CHARACTER SETS
 There are three types of characters in the input file:
 .Bl -tag -width "control characters"
@@ -1843,8 +1679,6 @@ end character in an ANSI color escape sequence
 (default "0123456789;[?!"'#%()*+\ ").
 .It Ev LESSBINFMT
 Format for displaying non-printable, non-control characters.
-.It Ev LESSCLOSE
-Command line to invoke the (optional) input-postprocessor.
 .It Ev LESSEDIT
 Editor prototype string (used for the v command).
 See discussion under
@@ -1874,8 +1708,6 @@ Prefix which
 will add before each metacharacter in a command sent to the shell.
 If LESSMETAESCAPE is an empty string, commands containing
 metacharacters will not be passed to the shell.
-.It Ev LESSOPEN
-Command line to invoke the (optional) input-preprocessor.
 .It Ev LESSSECURE
 Runs less in "secure" mode.
 See discussion under
index 259ade8..012c0b6 100644 (file)
@@ -165,7 +165,6 @@ extern int abort_sigs(void);
 /* filestate flags */
 #define        CH_CANSEEK      001
 #define        CH_KEEPOPEN     002
-#define        CH_POPENED      004
 #define        CH_HELPFILE     010
 #define        CH_NODATA       020     /* Special case for zero length files */
 
index 2bc53df..8bcae61 100644 (file)
                   Use a lesskey file.
   -K            --quit-on-intr
                   Exit less in response to ctrl-C.
-  -L  ........  --no-lessopen
-                  Ignore the LESSOPEN environment variable.
   -m  -M  ....  --long-prompt  --LONG-PROMPT
                   Set prompt style.
   -n  -N  ....  --line-numbers  --LINE-NUMBERS
index a29999e..926d1d4 100644 (file)
@@ -53,7 +53,6 @@ extern int    quit_if_one_screen;
 extern int     quit_at_eof;
 extern int     pr_type;
 extern int     hilite_search;
-extern int     use_lessopen;
 extern int     no_init;
 extern int     top_scroll;
 extern int     errmsgs;
@@ -131,9 +130,6 @@ main(int argc, char *argv[])
                /* do not highlight search terms */
                hilite_search = OPT_OFF;
 
-               /* do not use LESSOPEN */
-               use_lessopen = OPT_OFF;
-
                /* do not set init strings to terminal */
                no_init = OPT_ON;
 
index 483f4c1..301976e 100644 (file)
@@ -236,8 +236,8 @@ static struct loption option[] = {
        { 'L', &L__optname,
                BOOL, OPT_ON, &use_lessopen, NULL,
                {
-                       "Don't use the LESSOPEN filter",
-                       "Use the LESSOPEN filter",
+                       "(ignored)",
+                       "(ignored)",
                        NULL
                }
        },