CVE-2015-1335 (C, published 2015-10-01T20:59Z)
Description: value: lxc-start in lxc before 1.0.8 and 1.1.x before 1.1.4 allows local container administrators to escape AppArmor confinement via a symlink attack on a (1) mount target or (2) bind mount source.
Severity: HIGH (CVSS v3 base score: )
Weakness: CWE-59 \u2014 Improper Link Resolution Before File Access ('Link Following')
The product attempts to access a file based on the filename, but it does not properly prevent that filename from identifying a link or shortcut that resolves to an unintended resource.
Repository: https://github.com/lxc/lxc
Files changed:
doc/lxc.container.conf.sgml.in
src/lxc/cgfs.c
src/lxc/cgmanager.c
src/lxc/conf.c
src/lxc/utils.c
src/lxc/utils.h
src/tests/Makefile.am
src/tests/lxc-test-symlink
Commit message: CVE-2015-1335: Protect container mounts against symlinks

When a container starts up, lxc sets up the container's inital fstree
by doing a bunch of mounting, guided by the container configuration
file.  The container config is owned by the admin or user on the host,
so we do not try to guard against bad entries.  However, since the
mount target is in the container, it's possible that the container admin
could divert the mount with symbolic links.  This could bypass proper
container startup (i.e. confinement of a root-owned container by the
restrictive apparmor policy, by diverting the required write to
/proc/self/attr/current), or bypass the (path-based) apparmor policy
by diverting, say, /proc to /mnt in the container.

To prevent this,

1. do not allow mounts to paths containing symbolic links

2. do not allow bind mounts from relative paths containing symbolic
links.

Details:

Define safe_mount which ensures that the container has not inserted any
symbolic links into any mount targets for mounts to be done during
container setup.

The host's mount path may contain symbolic links.  As it is under the
control of the administrator, that's ok.  So safe_mount begins the check
for symbolic links after the rootfs->mount, by opening that directory.

It opens each directory along the path using openat() relative to the
parent directory using O_NOFOLLOW.  When the target is reached, it
mounts onto /proc/self/fd/<targetfd>.

Use safe_mount() in mount_entry(), when mounting container proc,
and when needed.  In particular, safe_mount() need not be used in
any case where:

1. the mount is done in the container's namespace
2. the mount is for the container's rootfs
3. the mount is relative to a tmpfs or proc/sysfs which we have
   just safe_mount()ed ourselves

Since we were using proc/net as a temporary placeholder for /proc/sys/net
during container startup, and proc/net is a symbolic link, use proc/tty
instead.

Update the lxc.container.conf manpage with details about the new
restrictions.

Finally, add a testcase to test some symbolic link possibilities.

Reported-by: Roman Fiedler
Signed-off-by: Serge Hallyn <serge.hallyn@ubuntu.com>
Acked-by: Stéphane Graber <stgraber@ubuntu.com>


Vulnerable code:
	r = mount("cgroup_root", path, "tmpfs", MS_NOSUID|MS_NODEV|MS_NOEXEC|MS_RELATIME, "size=10240k,mode=755");
	if (mount("cgroup", cgpath, "tmpfs", 0, "size=10000,mode=755")) {
	if (mount(dirname, cgpath, "none", MS_BIND, 0)) {
		{ LXC_AUTO_PROC_MASK, LXC_AUTO_PROC_MIXED, "%r/proc/sys/net",                                   "%r/proc/net",                  NULL,       MS_BIND,                        NULL },
		{ LXC_AUTO_PROC_MASK, LXC_AUTO_PROC_MIXED, "%r/proc/net",                                       "%r/proc/sys/net",              NULL,       MS_MOVE,                        NULL },
			r = mount(source, destination, default_mounts[i].fstype, mflags, default_mounts[i].options);
	if (mount("none", path, "tmpfs", 0, "size=100000,mode=755")) {
			if (mount(hostpath, path, 0, MS_BIND, NULL) != 0) {
	if (mount(console->name, path, "none", MS_BIND, 0)) {
	if (mount(console->name, lxcpath, "none", MS_BIND, 0)) {
		       const char *data, int optional)
	if (mount(fsname, target, fstype, mountflags & ~MS_REMOUNT, data)) {
			  mountflags | MS_REMOUNT, data)) {
                 const char* path)
			  mntflags, mntdata, optional);
  return mount_entry_on_generic(mntent, mntent->mnt_dir);
	return mount_entry_on_generic(mntent, path);
	return mount_entry_on_generic(mntent, path);
	ret = mount(path, destpath, "none", MS_BIND, NULL);
	if (mount("proc", path, "proc", 0, NULL))

Fixed code:
      <para>
	NOTE - LXC will generally ensure that mount targets and relative
	bind-mount sources are properly confined under the container
	root, to avoid attacks involving over-mounting host directories
	and files.  (Symbolic links in absolute mount sources are ignored)
	However, if the container configuration first mounts a directory which
	is under the control of the container user, such as /home/joe, into
        the container at some <filename>path</filename>, and then mounts
        under <filename>path</filename>, then a TOCTTOU attack would be
        possible where the container user modifies a symbolic link under
        his home directory at just the right time.
      </para>
	r = safe_mount("cgroup_root", path, "tmpfs",
			MS_NOSUID|MS_NODEV|MS_NOEXEC|MS_RELATIME,
			"size=10240k,mode=755",
			root);
	if (safe_mount("cgroup", cgpath, "tmpfs", 0, "size=10000,mode=755", root)) {
	if (safe_mount(dirname, cgpath, "none", MS_BIND, 0, root)) {
		/* proc/tty is used as a temporary placeholder for proc/sys/net which we'll move back in a few steps */
		{ LXC_AUTO_PROC_MASK, LXC_AUTO_PROC_MIXED, "%r/proc/sys/net",                                   "%r/proc/tty",                  NULL,       MS_BIND,                        NULL },
		{ LXC_AUTO_PROC_MASK, LXC_AUTO_PROC_MIXED, "%r/proc/tty",                                       "%r/proc/sys/net",              NULL,       MS_MOVE,                        NULL },
			r = safe_mount(source, destination, default_mounts[i].fstype, mflags, default_mounts[i].options, conf->rootfs.path ? conf->rootfs.mount : NULL);
	if (safe_mount("none", path, "tmpfs", 0, "size=100000,mode=755",
				rootfs->path ? rootfs->mount : NULL)) {
			if (safe_mount(hostpath, path, 0, MS_BIND, NULL,
						rootfs->path ? rootfs->mount : NULL) != 0) {
	if (safe_mount(console->name, path, "none", MS_BIND, 0, rootfs->mount)) {
	if (safe_mount(console->name, lxcpath, "none", MS_BIND, 0, rootfs->mount)) {
		       const char *data, int optional, const char *rootfs)
	if (safe_mount(fsname, target, fstype, mountflags & ~MS_REMOUNT, data, rootfs)) {
			  mountflags | MS_REMOUNT, data) < 0) {
                 const char* path, const char *rootfs)
			  mntflags, mntdata, optional, rootfs);
  return mount_entry_on_generic(mntent, mntent->mnt_dir, NULL);
	return mount_entry_on_generic(mntent, path, rootfs->mount);
	return mount_entry_on_generic(mntent, path, rootfs);
	ret = safe_mount(path, destpath, "none", MS_BIND, NULL, conf->rootfs.mount);
/*
 * @path:    a pathname where / replaced with '\0'.
 * @offsetp: pointer to int showing which path segment was last seen.
 *           Updated on return to reflect the next segment.
 * @fulllen: full original path length.
 * Returns a pointer to the next path segment, or NULL if done.
 */
static char *get_nextpath(char *path, int *offsetp, int fulllen)
{
	int offset = *offsetp;

	if (offset >= fulllen)
		return NULL;

	while (path[offset] != '\0' && offset < fulllen)
		offset++;
	while (path[offset] == '\0' && offset < fulllen)
		offset++;

	*offsetp = offset;
	return (offset < fulllen) ? &path[offset] : NULL;
}

/*
 * Check that @subdir is a subdir of @dir.  @len is the length of
 * @dir (to avoid having to recalculate it).
 */
static bool is_subdir(const char *subdir, const char *dir, size_t len)
{
	size_t subdirlen = strlen(subdir);

	if (subdirlen < len)
		return false;
	if (strncmp(subdir, dir, len) != 0)
		return false;
	if (dir[len-1] == '/')
		return true;
	if (subdir[len] == '/' || subdirlen == len)
		return true;
	return false;
}

/*
 * Check if the open fd is a symlink.  Return -ELOOP if it is.  Return
 * -ENOENT if we couldn't fstat.  Return 0 if the fd is ok.
 */
static int check_symlink(int fd)
{
	struct stat sb;
	int ret = fstat(fd, &sb);
	if (ret < 0)
		return -ENOENT;
	if (S_ISLNK(sb.st_mode))
		return -ELOOP;
	return 0;
}

/*
 * Open a file or directory, provided that it contains no symlinks.
 *
 * CAVEAT: This function must not be used for other purposes than container
 * setup before executing the container's init
 */
static int open_if_safe(int dirfd, const char *nextpath)
{
	int newfd = openat(dirfd, nextpath, O_RDONLY | O_NOFOLLOW);
	if (newfd >= 0) // was not a symlink, all good
		return newfd;

	if (errno == ELOOP)
		return newfd;

	if (errno == EPERM || errno == EACCES) {
		/* we're not root (cause we got EPERM) so
		   try opening with O_PATH */
		newfd = openat(dirfd, nextpath, O_PATH | O_NOFOLLOW);
		if (newfd >= 0) {
			/* O_PATH will return an fd for symlinks.  We know
			 * nextpath wasn't a symlink at last openat, so if fd
			 * is now a link, then something * fishy is going on
			 */
			int ret = check_symlink(newfd);
			if (ret < 0) {
				close(newfd);
				newfd = ret;
			}
		}
	}

	return newfd;
}

/*
 * Open a path intending for mounting, ensuring that the final path
 * is inside the container's rootfs.
 *
 * CAVEAT: This function must not be used for other purposes than container
 * setup before executing the container's init
 *
 * @target: path to be opened
 * @prefix_skip: a part of @target in which to ignore symbolic links.  This
 * would be the container's rootfs.
 *
 * Return an open fd for the path, or <0 on error.
 */
static int open_without_symlink(const char *target, const char *prefix_skip)
{
	int curlen = 0, dirfd, fulllen, i;
	char *dup = NULL;

	fulllen = strlen(target);

	/* make sure prefix-skip makes sense */
	if (prefix_skip) {
		curlen = strlen(prefix_skip);
		if (!is_subdir(target, prefix_skip, curlen)) {
			ERROR("WHOA there - target '%s' didn't start with prefix '%s'",
				target, prefix_skip);
			return -EINVAL;
		}
		/*
		 * get_nextpath() expects the curlen argument to be
		 * on a  (turned into \0) / or before it, so decrement
		 * curlen to make sure that happens
		 */
		if (curlen)
			curlen--;
	} else {
		prefix_skip = "/";
		curlen = 0;
	}

	/* Make a copy of target which we can hack up, and tokenize it */
	if ((dup = strdup(target)) == NULL) {
		SYSERROR("Out of memory checking for symbolic link");
		return -ENOMEM;
	}
	for (i = 0; i < fulllen; i++) {
		if (dup[i] == '/')
			dup[i] = '\0';
	}

	dirfd = open(prefix_skip, O_RDONLY);
	if (dirfd < 0)
		goto out;
	while (1) {
		int newfd, saved_errno;
		char *nextpath;

		if ((nextpath = get_nextpath(dup, &curlen, fulllen)) == NULL)
			goto out;
		newfd = open_if_safe(dirfd, nextpath);
		saved_errno = errno;
		close(dirfd);
		dirfd = newfd;
		if (newfd < 0) {
			errno = saved_errno;
			if (errno == ELOOP)
				SYSERROR("%s in %s was a symbolic link!", nextpath, target);
			else
				SYSERROR("Error examining %s in %s", nextpath, target);
			goto out;
		}
	}

out:
	free(dup);
	return dirfd;
}

/*
 * Safely mount a path into a container, ensuring that the mount target
 * is under the container's @rootfs.  (If @rootfs is NULL, then the container
 * uses the host's /)
 *
 * CAVEAT: This function must not be used for other purposes than container
 * setup before executing the container's init
 */
int safe_mount(const char *src, const char *dest, const char *fstype,
		unsigned long flags, const void *data, const char *rootfs)
{
	int srcfd = -1, destfd, ret, saved_errno;
	char srcbuf[50], destbuf[50]; // only needs enough for /proc/self/fd/<fd>
	const char *mntsrc = src;

	if (!rootfs)
		rootfs = "";

	/* todo - allow symlinks for relative paths if 'allowsymlinks' option is passed */
	if (flags & MS_BIND && src && src[0] != '/') {
		INFO("this is a relative bind mount");
		srcfd = open_without_symlink(src, NULL);
		if (srcfd < 0)
			return srcfd;
		ret = snprintf(srcbuf, 50, "/proc/self/fd/%d", srcfd);
		if (ret < 0 || ret > 50) {
			close(srcfd);
			ERROR("Out of memory");
			return -EINVAL;
		}
		mntsrc = srcbuf;
	}

	destfd = open_without_symlink(dest, rootfs);
	if (destfd < 0) {
		if (srcfd != -1)
			close(srcfd);
		return destfd;
	}

	ret = snprintf(destbuf, 50, "/proc/self/fd/%d", destfd);
	if (ret < 0 || ret > 50) {
		if (srcfd != -1)
			close(srcfd);
		close(destfd);
		ERROR("Out of memory");
		return -EINVAL;
	}

	ret = mount(mntsrc, destbuf, fstype, flags, data);
	saved_errno = errno;
	if (srcfd != -1)
		close(srcfd);
	close(destfd);
	if (ret < 0) {
		errno = saved_errno;
		SYSERROR("Failed to mount %s onto %s", src, dest);
		return ret;
	}

	return 0;
}

	if (safe_mount("proc", path, "proc", 0, NULL, rootfs) < 0)
int safe_mount(const char *src, const char *dest, const char *fstype,
		unsigned long flags, const void *data, const char *rootfs);
	lxc-test-symlink \
	lxc-test-symlink \
#!/bin/bash

set -ex

# lxc: linux Container library

# Authors:
# Serge Hallyn <serge.hallyn@ubuntu.com>
#
# This is a regression test for symbolic links

dirname=`mktemp -d`
fname=`mktemp`
fname2=`mktemp`

lxcpath=/var/lib/lxcsym1

cleanup() {
	lxc-destroy -P $lxcpath -f -n symtest1 || true
	rm -f $lxcpath
	rmdir $dirname || true
	rm -f $fname || true
	rm -f $fname2 || true
}

trap cleanup EXIT SIGHUP SIGINT SIGTERM

testrun() {
	expected=$1
	run=$2
	pass="pass"
	lxc-start -P $lxcpath -n symtest1 -l trace -o $lxcpath/log || pass="fail"
	[ $pass = "pass" ] && lxc-wait -P $lxcpath -n symtest1 -t 10 -s RUNNING || pass="fail"
	if [ "$pass" != "$expected" ]; then
		echo "Test $run: expected $expected but container did not.  Start log:"
		cat $lxcpath/log
		echo "FAIL: Test $run: expected $expected but container did not."
		false
	fi
	lxc-stop -P $lxcpath -n symtest1 -k || true
}

# make lxcpath a symlink - this should NOT cause failure
ln -s /var/lib/lxc $lxcpath

lxc-destroy -P $lxcpath -f -n symtest1 || true
lxc-create -P $lxcpath -t busybox -n symtest1

cat >> /var/lib/lxc/symtest1/config << EOF
lxc.mount.entry = $dirname opt/xxx/dir none bind,create=dir
lxc.mount.entry = $fname opt/xxx/file none bind,create=file
lxc.mount.entry = $fname2 opt/xxx/file2 none bind
EOF

# Regular - should succeed
mkdir -p /var/lib/lxc/symtest1/rootfs/opt/xxx
touch /var/lib/lxc/symtest1/rootfs/opt/xxx/file2
testrun pass 1

# symlink - should fail
rm -rf /var/lib/lxc/symtest1/rootfs/opt/xxx
mkdir -p /var/lib/lxc/symtest1/rootfs/opt/xxx2
ln -s /var/lib/lxc/symtest1/rootfs/opt/xxx2 /var/lib/lxc/symtest1/rootfs/opt/xxx
touch /var/lib/lxc/symtest1/rootfs/opt/xxx/file2
testrun fail 2

# final final symlink - should fail
rm -rf $lxcpath/symtest1/rootfs/opt/xxx
mkdir -p $lxcpath/symtest1/rootfs/opt/xxx
mkdir -p $lxcpath/symtest1/rootfs/opt/xxx/dir
touch $lxcpath/symtest1/rootfs/opt/xxx/file
touch $lxcpath/symtest1/rootfs/opt/xxx/file2src
ln -s $lxcpath/symtest1/rootfs/opt/xxx/file2src $lxcpath/symtest1/rootfs/opt/xxx/file2
testrun fail 3

# Ideally we'd also try a loop device, but that won't work in nested containers
# anyway - TODO

# what about /proc itself

rm -rf $lxcpath/symtest1/rootfs/opt/xxx
mkdir -p $lxcpath/symtest1/rootfs/opt/xxx
touch $lxcpath/symtest1/rootfs/opt/xxx/file2
mv $lxcpath/symtest1/rootfs/proc $lxcpath/symtest1/rootfs/proc1
ln -s $lxcpath/symtest1/rootfs/proc1 $lxcpath/symtest1/rootfs/proc
testrun fail 4

echo "all tests passed"