Fix potential double free for rules that include a CHROOT= option.

If a rule with a CHROOT= option matches the user, host and runas,
the user_cmnd variable could be freed twice.

7 files changed, 71 insertions, 11 deletions

diff --git a/MANIFEST b/MANIFEST
index be7dad1bf..839dc8c2f 100644
--- a/MANIFEST
+++ b/MANIFEST
@@ -1052,6 +1052,8 @@ plugins/sudoers/regress/testsudoers/test19.sh
 plugins/sudoers/regress/testsudoers/test2.inc
 plugins/sudoers/regress/testsudoers/test2.out.ok
 plugins/sudoers/regress/testsudoers/test2.sh
+plugins/sudoers/regress/testsudoers/test20.out.ok
+plugins/sudoers/regress/testsudoers/test20.sh
 plugins/sudoers/regress/testsudoers/test3.out.ok
 plugins/sudoers/regress/testsudoers/test3.sh
 plugins/sudoers/regress/testsudoers/test4.out.ok
diff --git a/plugins/sudoers/match_command.c b/plugins/sudoers/match_command.c
index a834c60e4..884e3d183 100644
--- a/plugins/sudoers/match_command.c
+++ b/plugins/sudoers/match_command.c
@@ -818,12 +818,16 @@ command_matches(const char *sudoers_cmnd, const char *sudoers_args,
 	/* Rule-specific runchroot, reset user_cmnd and user_stat. */
 	int status;
 
+	/* Save old user_cmnd first, set_cmnd_path() will free it. */
 	saved_user_cmnd = user_cmnd;
+	user_cmnd = NULL;
 	if (user_stat != NULL)
 	    saved_user_stat = *user_stat;
 	status = set_cmnd_path(runchroot);
-	if (status != FOUND)
+	if (status != FOUND) {
+	    user_cmnd = saved_user_cmnd;
 	    saved_user_cmnd = NULL;
+	}
 	if (info != NULL)
 	    info->status = status;
     }
diff --git a/plugins/sudoers/regress/fuzz/fuzz_sudoers.c b/plugins/sudoers/regress/fuzz/fuzz_sudoers.c
index 5c38a8be2..15dc72a21 100644
--- a/plugins/sudoers/regress/fuzz/fuzz_sudoers.c
+++ b/plugins/sudoers/regress/fuzz/fuzz_sudoers.c
@@ -45,6 +45,9 @@ static int fuzz_conversation(int num_msgs, const struct sudo_conv_message msgs[]
 static int fuzz_printf(int msg_type, const char *fmt, ...);
 int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size);
 
+/* For set_cmnd_path() */
+static const char *orig_cmnd;
+
 /* Required to link with parser. */
 struct sudo_user sudo_user;
 struct passwd *list_pw;
@@ -104,8 +107,13 @@ init_envtables(void)
 int
 set_cmnd_path(const char *runchroot)
 {
-    /* Cannot return FOUND without also setting user_cmnd to a new value. */
-    return NOT_FOUND;
+    /* Reallocate user_cmnd to catch bugs in command_matches(). */
+    char *new_cmnd = strdup(orig_cmnd);
+    if (new_cmnd == NULL)
+        return NOT_FOUND_ERROR;
+    free(user_cmnd);
+    user_cmnd = new_cmnd;
+    return FOUND;
 }
 
 /* STUB */
@@ -277,11 +285,12 @@ LLVMFuzzerTestOneInput(const uint8_t *data, size_t size)
 
     /* The minimum needed to perform matching (user_cmnd must be dynamic). */
     user_host = user_shost = user_runhost = user_srunhost = (char *)"localhost";
-    user_cmnd = strdup("/usr/bin/id");
+    orig_cmnd = (char *)"/usr/bin/id";
+    user_cmnd = strdup(orig_cmnd);
     if (user_cmnd == NULL)
 	goto done;
     user_args = (char *)"-u";
-    user_base = (char *)"id";
+    user_base = sudo_basename(user_cmnd);
 
     /* Add a fake network interfaces. */
     interfaces = get_interfaces();
diff --git a/plugins/sudoers/regress/testsudoers/test20.out.ok b/plugins/sudoers/regress/testsudoers/test20.out.ok
new file mode 100644
index 000000000..95f4c31fc
--- /dev/null
+++ b/plugins/sudoers/regress/testsudoers/test20.out.ok
@@ -0,0 +1,15 @@
+Parses OK
+
+Entries for user root:
+
+ALL = CHROOT=/ /bin/ls
+	host  matched
+	runas matched
+	cmnd  allowed
+
+ALL = CWD=/ /bin/pwd
+	host  matched
+	runas matched
+	cmnd  allowed
+
+Command allowed
diff --git a/plugins/sudoers/regress/testsudoers/test20.sh b/plugins/sudoers/regress/testsudoers/test20.sh
new file mode 100644
index 000000000..432517529
--- /dev/null
+++ b/plugins/sudoers/regress/testsudoers/test20.sh
@@ -0,0 +1,18 @@
+#!/bin/sh
+#
+# Verify CHROOT and CWD support
+# This will catch an unpatched double-free in set_cmnd_path() under ASAN.
+#
+
+: ${TESTSUDOERS=testsudoers}
+
+exec 2>&1
+
+# Exercise double free of user_cmnd in set_cmnd_path() under ASAN.
+# We need more than one rule where the last rule matches and has CHROOT.
+$TESTSUDOERS root /bin/ls <<'EOF'
+root ALL = CWD=/ /bin/pwd
+root ALL = CHROOT=/ /bin/ls
+EOF
+
+exit 0
diff --git a/plugins/sudoers/testsudoers.c b/plugins/sudoers/testsudoers.c
index ecc80820f..3eea3494b 100644
--- a/plugins/sudoers/testsudoers.c
+++ b/plugins/sudoers/testsudoers.c
@@ -82,6 +82,7 @@ extern int (*trace_print)(const char *msg);
  */
 struct sudo_user sudo_user;
 struct passwd *list_pw;
+static const char *orig_cmnd;
 static char *runas_group, *runas_user;
 
 #if defined(SUDO_DEVEL) && defined(__OpenBSD__)
@@ -203,14 +204,18 @@ main(int argc, char *argv[])
 	if (!dflag)
 	    usage();
 	user_name = argc ? *argv++ : (char *)"root";
-	user_cmnd = user_base = (char *)"true";
+	orig_cmnd = "true";
 	argc = 0;
     } else {
 	user_name = *argv++;
-	user_cmnd = *argv++;
-	user_base = sudo_basename(user_cmnd);
+	orig_cmnd = *argv++;
 	argc -= 2;
     }
+    user_cmnd = strdup(orig_cmnd);
+    if (user_cmnd == NULL)
+	sudo_fatalx(U_("%s: %s"), __func__, U_("unable to allocate memory"));
+    user_base = sudo_basename(user_cmnd);
+
     if ((sudo_user.pw = sudo_getpwnam(user_name)) == NULL)
 	sudo_fatalx(U_("unknown user %s"), user_name);
 
@@ -509,8 +514,13 @@ init_eventlog_config(void)
 int
 set_cmnd_path(const char *runchroot)
 {
-    /* Cannot return FOUND without also setting user_cmnd to a new value. */
-    return NOT_FOUND;
+    /* Reallocate user_cmnd to catch bugs in command_matches(). */
+    char *new_cmnd = strdup(orig_cmnd);
+    if (new_cmnd == NULL)
+	return NOT_FOUND_ERROR;
+    free(user_cmnd);
+    user_cmnd = new_cmnd;
+    return FOUND;
 }
 
 static bool
diff --git a/plugins/sudoers/visudo.c b/plugins/sudoers/visudo.c
index 425071afd..43151d7df 100644
--- a/plugins/sudoers/visudo.c
+++ b/plugins/sudoers/visudo.c
@@ -260,7 +260,9 @@ main(int argc, char *argv[])
     }
 
     /* Mock up a fake sudo_user struct. */
-    user_cmnd = user_base = (char *)"";
+    user_cmnd = user_base = strdup("true");
+    if (user_cmnd == NULL)
+	sudo_fatalx(U_("%s: %s"), __func__, U_("unable to allocate memory"));
     if (geteuid() == 0) {
 	const char *user = getenv("SUDO_USER");
 	if (user != NULL && *user != '\0')