From: Todd C. Miller <Todd.Miller@courtesan.com>
Date: Tue, 25 Jan 2011 14:53:57 +0000 (-0500)
Subject: Perform command escaping for "sudo -s" and "sudo -i" after validating
X-Git-Tag: SUDO_1_8_0~55
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=fc3848166320e861b7705515a74a4e2025a900d6;p=sudo

Perform command escaping for "sudo -s" and "sudo -i" after validating
sudoers so the sudoers entries don't need to have all the backslashes.
---

diff --git a/NEWS b/NEWS
index 0beceeea3..fe3c02032 100644
--- a/NEWS
+++ b/NEWS
@@ -64,6 +64,13 @@ What's new in Sudo 1.7.5?
 
  * The I/O log directory may now be specified in the sudoers file.
 
+ * Sudo will no longer refuse to run if the sudoers file is writable
+   by root.
+
+ * Sudo now performs command line escaping for "sudo -s" and "sudo -i"
+   after validating the command so the sudoers entries do not need
+   to include the backslashes.
+
 What's new in Sudo 1.7.4p6?
 
  * A bug has been fixed in the I/O logging support that could cause
diff --git a/src/parse_args.c b/src/parse_args.c
index f9b9ce289..93c521828 100644
--- a/src/parse_args.c
+++ b/src/parse_args.c
@@ -382,38 +382,17 @@ parse_args(int argc, char **argv, int *nargc, char ***nargv, char ***settingsp,
 	    memcpy(av + 1, argv, argc * sizeof(char *));
 	} else {
 	    /* shell -c "command" */
-	    size_t cmnd_size = 1024;
-	    char *cmnd, *src, *dst, **ap;
-
-	    cmnd = dst = emalloc(cmnd_size);
-	    for (ap = argv; *ap != NULL; ap++) {
-		for (src = *ap; *src != '\0'; src++) {
-		    /* reserve room for an escaped char + space */
-		    if (cmnd_size < (dst - cmnd) + 3) {
-			char *new_cmnd;
-			cmnd_size <<= 1;
-			new_cmnd = erealloc(cmnd, cmnd_size);
-			dst = new_cmnd + (dst - cmnd);
-			cmnd = new_cmnd;
-		    }
-		    if (isalnum((unsigned char)*src) || *src == '_' || *src == '-') {
-			*dst++ = *src;
-		    } else {
-			/* quote potential meta character */
-			*dst++ = '\\';
-			*dst++ = *src;
-		    }
-		}
-		*dst++ = ' ';
-	    }
-	    if (cmnd != dst)
-		dst--;  /* replace last space with a NUL */
-	    *dst = '\0';
-
+	    char *src, *dst, *end;
+	    size_t cmnd_size = (size_t) (argv[argc - 1] - argv[0]) +
+		    strlen(argv[argc - 1]) + 1;
 	    ac = 3;
 	    av = emalloc2(ac + 1, sizeof(char *));
 	    av[1] = "-c";
-	    av[2] = cmnd;
+	    av[2] = dst = emalloc(cmnd_size);
+	    src = argv[0];
+	    for (end = src + cmnd_size - 1; src < end; src++, dst++)
+		*dst = *src == '\0' ? ' ' : *src;
+	    *dst = '\0';
 	}
 	av[0] = (char *)user_details.shell; /* plugin may override shell */
 	av[ac] = NULL;
diff --git a/src/sudo.c b/src/sudo.c
index c7372dc6d..34b09c180 100644
--- a/src/sudo.c
+++ b/src/sudo.c
@@ -51,12 +51,13 @@
 #ifdef HAVE_UNISTD_H
 # include <unistd.h>
 #endif /* HAVE_UNISTD_H */
-#include <pwd.h>
+#include <ctype.h>
 #include <errno.h>
 #include <fcntl.h>
 #include <limits.h>
 #include <signal.h>
 #include <grp.h>
+#include <pwd.h>
 #if TIME_WITH_SYS_TIME
 # include <time.h>
 #endif
@@ -117,6 +118,7 @@ static int iolog_open(struct plugin_container *plugin, char * const settings[],
     int argc, char * const argv[], char * const user_env[]);
 static void iolog_close(struct plugin_container *plugin, int exit_status,
     int error);
+static char *escape_cmnd(const char *src);
 
 /* Policy plugin convenience functions. */
 static int policy_open(struct plugin_container *plugin, char * const settings[],
@@ -278,6 +280,12 @@ main(int argc, char *argv[], char *envp[])
 	    if (ISSET(command_details.flags, CD_SUDOEDIT)) {
 		exitcode = sudo_edit(&command_details, argv_out, user_env_out);
 	    } else {
+		if (ISSET(sudo_mode, MODE_SHELL)) {
+		    /* Escape meta chars if running a shell with args. */
+		    if (argv_out[1] != NULL && strcmp(argv_out[1], "-c") == 0 &&
+			argv_out[2] != NULL && argv_out[3] == NULL)
+			argv_out[2] = escape_cmnd(argv_out[2]);
+		}
 		exitcode = run_command(&command_details, argv_out, user_env_out);
 	    }
 	    /* The close method was called by sudo_edit/run_command. */
@@ -907,6 +915,30 @@ done:
     return rval;
 }
 
+/*
+ * Escape any non-alpha numeric or blank characters to make sure
+ * they are not interpreted specially by the shell.
+ */
+static char *
+escape_cmnd(const char *src)
+{
+    char *cmnd, *dst;
+
+    /* Worst case scenario, we have to escape everything. */
+    cmnd = dst = emalloc((2 * strlen(src)) + 1);
+    while (*src != '\0') {
+	if (!isalnum((unsigned char)*src) && !isspace((unsigned char)*src) &&
+	    *src != '_' && *src != '-') {
+	    /* quote potential meta character */
+	    *dst++ = '\\';
+	}
+	*dst++ = *src++;
+    }
+    *dst++ = '\0';
+
+    return cmnd;
+}
+
 /*
  * Run the command and wait for it to complete.
  */