From: guenther Date: Sun, 14 Apr 2024 18:11:54 +0000 (+0000) Subject: Delete support for the LESSOPEN and LESSCLOSE environment variables X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=c74702f8a020c6cfd3ae0dbd2b85bf100802950e;p=openbsd Delete support for the LESSOPEN and LESSCLOSE environment variables 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@ --- diff --git a/usr.bin/less/ch.c b/usr.bin/less/ch.c index d7c0aa34e90..8ca21eadcab 100644 --- a/usr.bin/less/ch.c +++ b/usr.bin/less/ch.c @@ -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; diff --git a/usr.bin/less/command.c b/usr.bin/less/command.c index 4fc19d949f7..d9464a32f19 100644 --- a/usr.bin/less/command.c +++ b/usr.bin/less/command.c @@ -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. diff --git a/usr.bin/less/edit.c b/usr.bin/less/edit.c index eff4d28dcf7..15a5be67e35 100644 --- a/usr.bin/less/edit.c +++ b/usr.bin/less/edit.c @@ -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; diff --git a/usr.bin/less/filename.c b/usr.bin/less/filename.c index dfe998fe9e3..0aaecac6197 100644 --- a/usr.bin/less/filename.c +++ b/usr.bin/less/filename.c @@ -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? */ diff --git a/usr.bin/less/funcs.h b/usr.bin/less/funcs.h index 2e707c3f606..ec86a310298 100644 --- a/usr.bin/less/funcs.h +++ b/usr.bin/less/funcs.h @@ -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); diff --git a/usr.bin/less/less.1 b/usr.bin/less/less.1 index 10811edcd89..fca7496f2e1 100644 --- a/usr.bin/less/less.1 +++ b/usr.bin/less/less.1 @@ -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 diff --git a/usr.bin/less/less.h b/usr.bin/less/less.h index 259ade82ed9..012c0b6abde 100644 --- a/usr.bin/less/less.h +++ b/usr.bin/less/less.h @@ -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 */ diff --git a/usr.bin/less/less.hlp b/usr.bin/less/less.hlp index 2bc53df374f..8bcae610dc1 100644 --- a/usr.bin/less/less.hlp +++ b/usr.bin/less/less.hlp @@ -149,8 +149,6 @@ 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 diff --git a/usr.bin/less/main.c b/usr.bin/less/main.c index a29999e02e6..926d1d4b1ab 100644 --- a/usr.bin/less/main.c +++ b/usr.bin/less/main.c @@ -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; diff --git a/usr.bin/less/opttbl.c b/usr.bin/less/opttbl.c index 483f4c1669b..301976e36ff 100644 --- a/usr.bin/less/opttbl.c +++ b/usr.bin/less/opttbl.c @@ -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 } },