From: tb <tb@openbsd.org>
Date: Sun, 14 Apr 2024 08:34:00 +0000 (+0000)
Subject: less: escape newlines in file names
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=d00ecc6f354c0069a7823d1f3b23fd011ad4316f;p=openbsd

less: escape newlines in file names

Newlines in a filename can lead to arbitrary code execution
https://marc.info/?l=oss-security&m=171292433330233&w=2
via LESSOPEN.  The diff is a straightforward adaptation of
https://github.com/gwsw/less/commit/007521ac3c95bc76

The better fix is deleting the misfeatures that are LESSOPEN
and LESSCLOSE which will happen in a separate commit.

diff looks good to guenther
---

diff --git a/usr.bin/less/filename.c b/usr.bin/less/filename.c
index 8b2dd9a02f4..dfe998fe9e3 100644
--- a/usr.bin/less/filename.c
+++ b/usr.bin/less/filename.c
@@ -106,6 +106,15 @@ metachar(char c)
 	return (strchr(metachars(), c) != NULL);
 }
 
+/*
+ * Must use quotes rather than escape characters for this meta character.
+ */
+static int
+must_quote(char c)
+{
+	return (c == '\n');
+}
+
 /*
  * Insert a backslash before each metacharacter in a string.
  */
@@ -136,6 +145,9 @@ shell_quote(const char *s)
 				 * doesn't support escape chars.  Use quotes.
 				 */
 				use_quotes = 1;
+			} else if (must_quote(*p)) {
+				/* Opening quote + character + closing quote. */
+				len += 3;
 			} else {
 				/*
 				 * Allow space for the escape char.
@@ -155,14 +167,19 @@ shell_quote(const char *s)
 	} else {
 		newstr = r = ecalloc(len, sizeof (char));
 		while (*s != '\0') {
-			if (metachar(*s)) {
-				/*
-				 * Add the escape char.
-				 */
+			if (!metachar(*s)) {
+				*r++ = *s++;
+			} else if (must_quote(*s)) {
+				/* Surround the character with quotes. */
+				*r++ = openquote;
+				*r++ = *s++;
+				*r++ = closequote;
+			} else {
+				/* Escape the character. */
 				(void) strlcpy(r, esc, newstr + len - p);
 				r += esclen;
+				*r++ = *s++;
 			}
-			*r++ = *s++;
 		}
 		*r = '\0';
 	}