From a7a7f45ee7d37f5bd9ccf10c405a51eba7027262 Mon Sep 17 00:00:00 2001
From: jamie <jamie@FreeBSD.org>
Date: Thu, 4 Nov 2010 18:40:29 +0000
Subject: Check paths for security:  path must be absolute.  mount paths must
 exist and have no symlinks beyond the jail's path itself.  consolelog must
 exist (apart from the final component) and have no   symlinks beyond the
 jail's path itself.

---
 usr.sbin/jail/command.c | 71 ++++++++++++++++++++++++++++++++++++++++++++++---
 usr.sbin/jail/jail.c    |  6 +++++
 2 files changed, 74 insertions(+), 3 deletions(-)

(limited to 'usr.sbin')

diff --git a/usr.sbin/jail/command.c b/usr.sbin/jail/command.c
index dfb7e4e..eca55e3 100644
--- a/usr.sbin/jail/command.c
+++ b/usr.sbin/jail/command.c
@@ -68,6 +68,8 @@ static int get_user_info(struct cfjail *j, const char *username,
 static void add_proc(struct cfjail *j, pid_t pid);
 static void clear_procs(struct cfjail *j);
 static struct cfjail *find_proc(pid_t pid);
+static int check_path(struct cfjail *j, const char *pname, const char *path,
+    int isfile);
 
 static struct cfjails sleeping = TAILQ_HEAD_INITIALIZER(sleeping);
 static struct cfjails runnable = TAILQ_HEAD_INITIALIZER(runnable);
@@ -84,7 +86,7 @@ run_command(struct cfjail *j, int *plimit, enum intparam comparam)
 	struct cfstring *comstring, *s;
 	login_cap_t *lcap;
 	char **argv;
-	char *cs, *addr, *comcs;
+	char *cs, *addr, *comcs, *devpath;
 	const char *jidstr, *conslog, *path, *ruleset, *term, *username;
 	size_t comlen;
 	pid_t pid;
@@ -197,12 +199,16 @@ run_command(struct cfjail *j, int *plimit, enum intparam comparam)
 		for (cs = strtok(comcs, " \t\f\v\r\n"); cs && argc < 4;
 		     cs = strtok(NULL, " \t\f\v\r\n"))
 			argv[argc++] = cs;
+		if (argc == 0)
+			return 0;
 		if (argc < 3) {
 			jail_warnx(j, "%s: %s: missing information",
 			    j->intparams[comparam]->name, comstring->s);
 			failed(j);
 			return -1;
 		}
+		if (check_path(j, j->intparams[comparam]->name, argv[1], 0) < 0)
+			return -1;
 		if (down) {
 			argv[4] = NULL;
 			argv[3] = argv[1];
@@ -230,11 +236,14 @@ run_command(struct cfjail *j, int *plimit, enum intparam comparam)
 			failed(j);
 			return -1;
 		}
+		devpath = alloca(strlen(path) + 5);
+		sprintf(devpath, "%s/dev", path);
+		if (check_path(j, "mount.devfs", devpath, 0) < 0)
+			return -1;
 		if (down) {
 			argv = alloca(3 * sizeof(char *));
 			*(const char **)&argv[0] = "/sbin/umount";
-			argv[1] = alloca(strlen(path) + 5);
-			sprintf(argv[1], "%s/dev", path);
+			argv[1] = devpath;
 			argv[2] = NULL;
 		} else {
 			argv = alloca(4 * sizeof(char *));
@@ -307,6 +316,8 @@ run_command(struct cfjail *j, int *plimit, enum intparam comparam)
 	consfd = 0;
 	if (injail &&
 	    (conslog = string_param(j->intparams[IP_EXEC_CONSOLELOG]))) {
+		if (check_path(j, "exec.consolelog", conslog, 1) < 0)
+			return -1;
 		consfd =
 		    open(conslog, O_WRONLY | O_CREAT | O_APPEND, DEFFILEMODE);
 		if (consfd < 0) {
@@ -666,3 +677,57 @@ get_user_info(struct cfjail *j, const char *username,
 	}
 	return 0;
 }
+
+/*
+ * Make sure a mount or consolelog path is a valid absolute pathname
+ * with no symlinks.
+ */
+static int
+check_path(struct cfjail *j, const char *pname, const char *path, int isfile)
+{
+	struct stat st;
+	char *tpath, *p;
+	const char *jailpath;
+	size_t jplen;
+
+	if (path[0] != '/') {
+		jail_warnx(j, "%s: %s: not an absolute pathname",
+		    pname, path);
+		failed(j);
+		return -1;
+	}
+	/*
+	 * Only check for symlinks in components below the jail's path,
+	 * since that's where the security risk lies.
+	 */
+	jailpath = string_param(j->intparams[KP_PATH]);
+	if (jailpath == NULL)
+		jailpath = "";
+	jplen = strlen(jailpath);
+	if (strncmp(path, jailpath, jplen) || path[jplen] != '/')
+		return 0;
+	tpath = alloca(strlen(path) + 1);
+	strcpy(tpath, path);
+	for (p = tpath + jplen; p != NULL; ) {
+		p = strchr(p + 1, '/');
+		if (p)
+			*p = '\0';
+		if (lstat(tpath, &st) < 0) {
+			if (errno == ENOENT && isfile && !p)
+				break;
+			jail_warnx(j, "%s: %s: %s", pname, tpath,
+			    strerror(errno));
+			failed(j);
+			return -1;
+		}
+		if (S_ISLNK(st.st_mode)) {
+			jail_warnx(j, "%s: %s is a symbolic link",
+			    pname, tpath);
+			failed(j);
+			return -1;
+		}
+		if (p)
+			*p = '/';
+	}
+	return 0;
+}
diff --git a/usr.sbin/jail/jail.c b/usr.sbin/jail/jail.c
index 0bc1bd1..dd1b92c 100644
--- a/usr.sbin/jail/jail.c
+++ b/usr.sbin/jail/jail.c
@@ -659,6 +659,12 @@ create_jail(struct cfjail *j)
 	 * gives.
 	 */
 	if ((path = string_param(j->intparams[KP_PATH]))) {
+		if (path[0] != '/') {
+			jail_warnx(j, "path %s: not an absolute pathname",
+			    path);
+			failed(j);
+			return -1;
+		}
 		if (stat(path, &st) < 0) {
 			jail_warnx(j, "path %s: %s", path, strerror(errno));
 			failed(j);
-- 
cgit v1.1