Escape control characters in log messages and "sudoreplay -l" output.

The log message contains user-controlled strings that could include
things like terminal control characters.  Space characters in the
command path are now also escaped.

Command line arguments that contain spaces are surrounded with
single quotes and any literal single quote or backslash characters
are escaped with a backslash.  This makes it possible to distinguish
multiple command line arguments from a single argument that contains
spaces.

Issue found by Matthieu Barjole and Victor Cutillas of Synacktiv
(https://synacktiv.com).

4 files changed, 168 insertions, 188 deletions

diff --git a/lib/eventlog/eventlog.c b/lib/eventlog/eventlog.c
index c0183d3d2..e5dae626a 100644
--- a/lib/eventlog/eventlog.c
+++ b/lib/eventlog/eventlog.c
@@ -1,7 +1,7 @@
 /*
  * SPDX-License-Identifier: ISC
  *
- * Copyright (c) 1994-1996, 1998-2021 Todd C. Miller <Todd.Miller@sudo.ws>
+ * Copyright (c) 1994-1996, 1998-2023 Todd C. Miller <Todd.Miller@sudo.ws>
  *
  * Permission to use, copy, modify, and distribute this software for any
  * purpose with or without fee is hereby granted, provided that the above
@@ -51,24 +51,13 @@
 #include "sudo_compat.h"
 #include "sudo_debug.h"
 #include "sudo_eventlog.h"
+#include "sudo_lbuf.h"
 #include "sudo_fatal.h"
 #include "sudo_gettext.h"
 #include "sudo_json.h"
 #include "sudo_queue.h"
 #include "sudo_util.h"
 
-#define	LL_HOST_STR	"HOST="
-#define	LL_TTY_STR	"TTY="
-#define	LL_CHROOT_STR	"CHROOT="
-#define	LL_CWD_STR	"PWD="
-#define	LL_USER_STR	"USER="
-#define	LL_GROUP_STR	"GROUP="
-#define	LL_ENV_STR	"ENV="
-#define	LL_CMND_STR	"COMMAND="
-#define	LL_TSID_STR	"TSID="
-#define	LL_EXIT_STR	"EXIT="
-#define	LL_SIGNAL_STR	"SIGNAL="
-
 #define IS_SESSID(s) ( \
     isalnum((unsigned char)(s)[0]) && isalnum((unsigned char)(s)[1]) && \
     (s)[2] == '/' && \
@@ -93,26 +82,28 @@ new_logline(int event_type, int flags, struct eventlog_args *args,
     const struct eventlog *evlog)
 {
     const struct eventlog_config *evl_conf = eventlog_getconf();
-    char *line = NULL, *evstr = NULL;
     const char *iolog_file;
     const char *tty, *tsid = NULL;
     char exit_str[(((sizeof(int) * 8) + 2) / 3) + 2];
     char sessid[7], offsetstr[64] = "";
-    size_t len = 0;
+    struct sudo_lbuf lbuf;
     int i;
     debug_decl(new_logline, SUDO_DEBUG_UTIL);
 
+    sudo_lbuf_init(&lbuf, NULL, 0, NULL, 0);
+
     if (ISSET(flags, EVLOG_RAW) || evlog == NULL) {
 	if (args->reason != NULL) {
 	    if (args->errstr != NULL) {
-		if (asprintf(&line, "%s: %s", args->reason, args->errstr) == -1)
-		    goto oom;
+		sudo_lbuf_append_esc(&lbuf, LBUF_ESC_CNTRL, "%s: %s",
+		    args->reason, args->errstr);
 	    } else {
-		if ((line = strdup(args->reason)) == NULL)
-		    goto oom;
+		sudo_lbuf_append_esc(&lbuf, LBUF_ESC_CNTRL, "%s", args->reason);
 	    }
+	    if (sudo_lbuf_error(&lbuf))
+		goto oom;
 	}
-	debug_return_str(line);
+	debug_return_str(lbuf.buf);
     }
 
     /* A TSID may be a sudoers-style session ID or a free-form string. */
@@ -150,169 +141,90 @@ new_logline(int event_type, int flags, struct eventlog_args *args,
     }
 
     /*
-     * Compute line length
+     * Format the log line as an lbuf, escaping control characters in
+     * octal form (#0nn).  Error checking (ENOMEM) is done at the end.
      */
-    if (args->reason != NULL)
-	len += strlen(args->reason) + 3;
-    if (args->errstr != NULL)
-	len += strlen(args->errstr) + 3;
-    if (evlog->submithost != NULL && !evl_conf->omit_hostname)
-	len += sizeof(LL_HOST_STR) + 2 + strlen(evlog->submithost);
-    if (tty != NULL)
-	len += sizeof(LL_TTY_STR) + 2 + strlen(tty);
-    if (evlog->runchroot != NULL)
-	len += sizeof(LL_CHROOT_STR) + 2 + strlen(evlog->runchroot);
-    if (evlog->runcwd != NULL)
-	len += sizeof(LL_CWD_STR) + 2 + strlen(evlog->runcwd);
-    if (evlog->runuser != NULL)
-	len += sizeof(LL_USER_STR) + 2 + strlen(evlog->runuser);
-    if (evlog->rungroup != NULL)
-	len += sizeof(LL_GROUP_STR) + 2 + strlen(evlog->rungroup);
-    if (tsid != NULL) {
-	len += sizeof(LL_TSID_STR) + 2 + strlen(tsid) + strlen(offsetstr);
-    }
-    if (evlog->env_add != NULL) {
-	size_t evlen = 0;
-	char * const *ep;
-
-	for (ep = evlog->env_add; *ep != NULL; ep++)
-	    evlen += strlen(*ep) + 1;
-	if (evlen != 0) {
-	    if ((evstr = malloc(evlen)) == NULL)
-		goto oom;
-	    ep = evlog->env_add;
-	    if (strlcpy(evstr, *ep, evlen) >= evlen)
-		goto toobig;
-	    while (*++ep != NULL) {
-		if (strlcat(evstr, " ", evlen) >= evlen ||
-		    strlcat(evstr, *ep, evlen) >= evlen)
-		    goto toobig;
-	    }
-	    len += sizeof(LL_ENV_STR) + 2 + evlen;
-	}
-    }
-    if (evlog->command != NULL) {
-	len += sizeof(LL_CMND_STR) - 1 + strlen(evlog->command);
-	if (evlog->argv != NULL && evlog->argv[0] != NULL) {
-	    for (i = 1; evlog->argv[i] != NULL; i++)
-		len += strlen(evlog->argv[i]) + 1;
-	}
-	if (event_type == EVLOG_EXIT) {
-	    if (evlog->signal_name != NULL)
-		len += sizeof(LL_SIGNAL_STR) + 2 + strlen(evlog->signal_name);
-	    if (evlog->exit_value != -1) {
-		(void)snprintf(exit_str, sizeof(exit_str), "%d", evlog->exit_value);
-		len += sizeof(LL_EXIT_STR) + 2 + strlen(exit_str);
-	    }
-	}
-    }
-
-    /*
-     * Allocate and build up the line.
-     */
-    if ((line = malloc(++len)) == NULL)
-	goto oom;
-    line[0] = '\0';
-
     if (args->reason != NULL) {
-	if (strlcat(line, args->reason, len) >= len ||
-	    strlcat(line, args->errstr ? " : " : " ; ", len) >= len)
-	    goto toobig;
+	sudo_lbuf_append_esc(&lbuf, LBUF_ESC_CNTRL, "%s%s", args->reason,
+	    args->errstr ? " : " : " ; ");
     }
     if (args->errstr != NULL) {
-	if (strlcat(line, args->errstr, len) >= len ||
-	    strlcat(line, " ; ", len) >= len)
-	    goto toobig;
+	sudo_lbuf_append_esc(&lbuf, LBUF_ESC_CNTRL, "%s ; ", args->errstr);
     }
     if (evlog->submithost != NULL && !evl_conf->omit_hostname) {
-	if (strlcat(line, LL_HOST_STR, len) >= len ||
-	    strlcat(line, evlog->submithost, len) >= len ||
-	    strlcat(line, " ; ", len) >= len)
-	    goto toobig;
+	sudo_lbuf_append_esc(&lbuf, LBUF_ESC_CNTRL, "HOST=%s ; ",
+	    evlog->submithost);
     }
     if (tty != NULL) {
-	if (strlcat(line, LL_TTY_STR, len) >= len ||
-	    strlcat(line, tty, len) >= len ||
-	    strlcat(line, " ; ", len) >= len)
-	    goto toobig;
+	sudo_lbuf_append_esc(&lbuf, LBUF_ESC_CNTRL, "TTY=%s ; ", tty);
     }
     if (evlog->runchroot != NULL) {
-	if (strlcat(line, LL_CHROOT_STR, len) >= len ||
-	    strlcat(line, evlog->runchroot, len) >= len ||
-	    strlcat(line, " ; ", len) >= len)
-	    goto toobig;
+	sudo_lbuf_append_esc(&lbuf, LBUF_ESC_CNTRL, "CHROOT=%s ; ",
+	    evlog->runchroot);
     }
     if (evlog->runcwd != NULL) {
-	if (strlcat(line, LL_CWD_STR, len) >= len ||
-	    strlcat(line, evlog->runcwd, len) >= len ||
-	    strlcat(line, " ; ", len) >= len)
-	    goto toobig;
+	sudo_lbuf_append_esc(&lbuf, LBUF_ESC_CNTRL, "PWD=%s ; ",
+	    evlog->runcwd);
     }
     if (evlog->runuser != NULL) {
-	if (strlcat(line, LL_USER_STR, len) >= len ||
-	    strlcat(line, evlog->runuser, len) >= len ||
-	    strlcat(line, " ; ", len) >= len)
-	    goto toobig;
+	sudo_lbuf_append_esc(&lbuf, LBUF_ESC_CNTRL, "USER=%s ; ",
+	    evlog->runuser);
     }
     if (evlog->rungroup != NULL) {
-	if (strlcat(line, LL_GROUP_STR, len) >= len ||
-	    strlcat(line, evlog->rungroup, len) >= len ||
-	    strlcat(line, " ; ", len) >= len)
-	    goto toobig;
+	sudo_lbuf_append_esc(&lbuf, LBUF_ESC_CNTRL, "GROUP=%s ; ",
+	    evlog->rungroup);
     }
     if (tsid != NULL) {
-	if (strlcat(line, LL_TSID_STR, len) >= len ||
-	    strlcat(line, tsid, len) >= len ||
-	    strlcat(line, offsetstr, len) >= len ||
-	    strlcat(line, " ; ", len) >= len)
-	    goto toobig;
-    }
-    if (evstr != NULL) {
-	if (strlcat(line, LL_ENV_STR, len) >= len ||
-	    strlcat(line, evstr, len) >= len ||
-	    strlcat(line, " ; ", len) >= len)
-	    goto toobig;
-	free(evstr);
-	evstr = NULL;
+	sudo_lbuf_append_esc(&lbuf, LBUF_ESC_CNTRL, "TSID=%s%s ; ", tsid,
+	    offsetstr);
+    }
+    if (evlog->env_add != NULL && evlog->env_add[0] != NULL) {
+	sudo_lbuf_append_esc(&lbuf, LBUF_ESC_CNTRL, "ENV=%s",
+	    evlog->env_add[0]);
+	for (i = 1; evlog->env_add[i] != NULL; i++) {
+	    sudo_lbuf_append_esc(&lbuf, LBUF_ESC_CNTRL, " %s",
+		evlog->env_add[i]);
+	}
     }
     if (evlog->command != NULL) {
-	if (strlcat(line, LL_CMND_STR, len) >= len)
-	    goto toobig;
-	if (strlcat(line, evlog->command, len) >= len)
-	    goto toobig;
+	sudo_lbuf_append_esc(&lbuf, LBUF_ESC_CNTRL|LBUF_ESC_BLANK,
+	    "COMMAND=%s", evlog->command);
 	if (evlog->argv != NULL && evlog->argv[0] != NULL) {
 	    for (i = 1; evlog->argv[i] != NULL; i++) {
-		if (strlcat(line, " ", len) >= len ||
-		    strlcat(line, evlog->argv[i], len) >= len)
-		    goto toobig;
+		sudo_lbuf_append(&lbuf, " ");
+		if (strchr(evlog->argv[i], ' ') != NULL) {
+		    /* Wrap args containing spaces in single quotes. */
+		    sudo_lbuf_append(&lbuf, "'");
+		    sudo_lbuf_append_esc(&lbuf, LBUF_ESC_CNTRL|LBUF_ESC_QUOTE,
+			"%s", evlog->argv[i]);
+		    sudo_lbuf_append(&lbuf, "'");
+		} else {
+		    /* Escape quotes here too for consistency. */
+		    sudo_lbuf_append_esc(&lbuf,
+			LBUF_ESC_CNTRL|LBUF_ESC_BLANK|LBUF_ESC_QUOTE,
+			"%s", evlog->argv[i]);
+		}
 	    }
 	}
 	if (event_type == EVLOG_EXIT) {
 	    if (evlog->signal_name != NULL) {
-		if (strlcat(line, " ; ", len) >= len ||
-		    strlcat(line, LL_SIGNAL_STR, len) >= len ||
-		    strlcat(line, evlog->signal_name, len) >= len)
-		    goto toobig;
+		sudo_lbuf_append_esc(&lbuf, LBUF_ESC_CNTRL, " ; SIGNAL=%s",
+		    evlog->signal_name);
 	    }
 	    if (evlog->exit_value != -1) {
-		if (strlcat(line, " ; ", len) >= len ||
-		    strlcat(line, LL_EXIT_STR, len) >= len ||
-		    strlcat(line, exit_str, len) >= len)
-		    goto toobig;
+		(void)snprintf(exit_str, sizeof(exit_str), "%d",
+		    evlog->exit_value);
+		sudo_lbuf_append_esc(&lbuf, LBUF_ESC_CNTRL, " ; EXIT=%s",
+		    exit_str);
 	    }
 	}
     }
-
-    debug_return_str(line);
+    if (!sudo_lbuf_error(&lbuf))
+	debug_return_str(lbuf.buf);
 oom:
-    free(evstr);
+    sudo_lbuf_destroy(&lbuf);
     sudo_warnx(U_("%s: %s"), __func__, U_("unable to allocate memory"));
     debug_return_str(NULL);
-toobig:
-    free(evstr);
-    free(line);
-    sudo_warnx(U_("internal error, %s overflow"), __func__);
-    debug_return_str(NULL);
 }
 
 static void
diff --git a/lib/iolog/iolog_json.c b/lib/iolog/iolog_json.c
index 5ea338e5d..6f384ea59 100644
--- a/lib/iolog/iolog_json.c
+++ b/lib/iolog/iolog_json.c
@@ -551,45 +551,6 @@ iolog_parse_json_object(struct json_object *object, struct eventlog *evlog)
 	}
     }
 
-    /* Merge cmd and argv as sudoreplay expects. */
-    if (evlog->command != NULL && evlog->argv != NULL && evlog->argv[0] != NULL) {
-	size_t len, bufsize = strlen(evlog->command) + 1;
-	char *cp, *buf;
-	int ac;
-
-	/* Skip argv[0], we use evlog->command instead. */
-	for (ac = 1; evlog->argv[ac] != NULL; ac++)
-	    bufsize += strlen(evlog->argv[ac]) + 1;
-
-	if ((buf = malloc(bufsize)) == NULL) {
-	    sudo_warnx(U_("%s: %s"), __func__, U_("unable to allocate memory"));
-	    goto done;
-	}
-	cp = buf;
-
-	len = strlcpy(cp, evlog->command, bufsize);
-	if (len >= bufsize)
-	    sudo_fatalx(U_("internal error, %s overflow"), __func__);
-	cp += len;
-	bufsize -= len;
-
-	for (ac = 1; evlog->argv[ac] != NULL; ac++) {
-	    if (bufsize < 2)
-		sudo_fatalx(U_("internal error, %s overflow"), __func__);
-	    *cp++ = ' ';
-	    bufsize--;
-
-	    len = strlcpy(cp, evlog->argv[ac], bufsize);
-	    if (len >= bufsize)
-		sudo_fatalx(U_("internal error, %s overflow"), __func__);
-	    cp += len;
-	    bufsize -= len;
-	}
-
-	free(evlog->command);
-	evlog->command = buf;
-    }
-
     ret = true;
 
 done:
diff --git a/lib/util/lbuf.c b/lib/util/lbuf.c
index 101982065..72bcac26f 100644
--- a/lib/util/lbuf.c
+++ b/lib/util/lbuf.c
@@ -95,6 +95,112 @@ sudo_lbuf_expand(struct sudo_lbuf *lbuf, unsigned int extra)
 }
 
 /*
+ * Escape a character in octal form (#0n) and store it as a string
+ * in buf, which must have at least 6 bytes available.
+ * Returns the length of buf, not counting the terminating NUL byte.
+ */
+static int
+escape(unsigned char ch, char *buf)
+{
+    const int len = ch < 0100 ? (ch < 010 ? 3 : 4) : 5;
+
+    /* Work backwards from the least significant digit to most significant. */
+    switch (len) {
+    case 5:
+	buf[4] = (ch & 7) + '0';
+	ch >>= 3;
+	FALLTHROUGH;
+    case 4:
+	buf[3] = (ch & 7) + '0';
+	ch >>= 3;
+	FALLTHROUGH;
+    case 3:
+	buf[2] = (ch & 7) + '0';
+	buf[1] = '0';
+	buf[0] = '#';
+	break;
+    }
+    buf[len] = '\0';
+
+    return len;
+}
+
+/*
+ * Parse the format and append strings, only %s and %% escapes are supported.
+ * Any non-printable characters are escaped in octal as #0nn.
+ */
+bool
+sudo_lbuf_append_esc_v1(struct sudo_lbuf *lbuf, int flags, const char *fmt, ...)
+{
+    unsigned int saved_len = lbuf->len;
+    bool ret = false;
+    const char *s;
+    va_list ap;
+    debug_decl(sudo_lbuf_append_esc, SUDO_DEBUG_UTIL);
+
+    if (sudo_lbuf_error(lbuf))
+	debug_return_bool(false);
+
+#define should_escape(ch) \
+    ((ISSET(flags, LBUF_ESC_CNTRL) && iscntrl((unsigned char)ch)) || \
+    (ISSET(flags, LBUF_ESC_BLANK) && isblank((unsigned char)ch)))
+#define should_quote(ch) \
+    (ISSET(flags, LBUF_ESC_QUOTE) && (ch == '\'' || ch == '\\'))
+
+    va_start(ap, fmt);
+    while (*fmt != '\0') {
+	if (fmt[0] == '%' && fmt[1] == 's') {
+	    if ((s = va_arg(ap, char *)) == NULL)
+		s = "(NULL)";
+	    while (*s != '\0') {
+		if (should_escape(*s)) {
+		    if (!sudo_lbuf_expand(lbuf, sizeof("#0177") - 1))
+			goto done;
+		    lbuf->len += escape(*s++, lbuf->buf + lbuf->len);
+		    continue;
+		}
+		if (should_quote(*s)) {
+		    if (!sudo_lbuf_expand(lbuf, 2))
+			goto done;
+		    lbuf->buf[lbuf->len++] = '\\';
+		    lbuf->buf[lbuf->len++] = *s++;
+		    continue;
+		}
+		if (!sudo_lbuf_expand(lbuf, 1))
+		    goto done;
+		lbuf->buf[lbuf->len++] = *s++;
+	    }
+	    fmt += 2;
+	    continue;
+	}
+	if (should_escape(*fmt)) {
+	    if (!sudo_lbuf_expand(lbuf, sizeof("#0177") - 1))
+		goto done;
+	    if (*fmt == '\'') {
+		lbuf->buf[lbuf->len++] = '\\';
+		lbuf->buf[lbuf->len++] = *fmt++;
+	    } else {
+		lbuf->len += escape(*fmt++, lbuf->buf + lbuf->len);
+	    }
+	    continue;
+	}
+	if (!sudo_lbuf_expand(lbuf, 1))
+	    goto done;
+	lbuf->buf[lbuf->len++] = *fmt++;
+    }
+    ret = true;
+
+done:
+    if (!ret)
+	lbuf->len = saved_len;
+    if (lbuf->size != 0)
+	lbuf->buf[lbuf->len] = '\0';
+    va_end(ap);
+
+    debug_return_bool(ret);
+}
+
+/*
  * Parse the format and append strings, only %s and %% escapes are supported.
  * Any characters in set are quoted with a backslash.
  */
diff --git a/lib/util/util.exp.in b/lib/util/util.exp.in
index 554904c5f..6c8130a02 100644
--- a/lib/util/util.exp.in
+++ b/lib/util/util.exp.in
@@ -100,6 +100,7 @@ sudo_json_init_v1
 sudo_json_init_v2
 sudo_json_open_array_v1
 sudo_json_open_object_v1
+sudo_lbuf_append_esc_v1
 sudo_lbuf_append_quoted_v1
 sudo_lbuf_append_v1
 sudo_lbuf_clearerr_v1