Index | Thread | Search

From:
Theo Buehler <tb@theobuehler.org>
Subject:
less: fix escaping of newlines in paths
To:
tech@openbsd.org
Date:
Sat, 13 Apr 2024 07:52:14 +0100

Download raw body.

Thread
If you use LESSOPEN, this can apparently lead to arbitrary code execution:
https://marc.info/?l=oss-security&m=171292433330233&w=2

The below is a straightforward adaptation of the fix from Jakub Wilk in
https://github.com/gwsw/less/commit/007521ac3c95bc76

By a weird coincidence, gdamore archived the less-fork repo a couple of
weeks ago: https://github.com/gdamore/less-fork

Index: filename.c
===================================================================
RCS file: /cvs/src/usr.bin/less/filename.c,v
diff -u -p -r1.29 filename.c
--- filename.c	28 Jun 2019 13:35:01 -0000	1.29
+++ filename.c	12 Apr 2024 16:17:01 -0000
@@ -107,6 +107,15 @@ metachar(char c)
 }
 
 /*
+ * 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.
  */
 char *
@@ -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';
 	}