CVE-2015-1340: Concurrent Execution using Shared Resource with Improper Synchronization ('Race Condition') (CWE-362)
value: LXD before version 0.19-0ubuntu5 doUidshiftIntoContainer() has an unsafe Chmod() call that races against the stat in the Filepath.Walk() function. A symbolic link created in that window could cause any file on the system to have any mode of the attacker's choice.
CWE description: The product contains a code sequence that can run concurrently with other code, and the code sequence requires temporary, exclusive access to a shared resource, but a timing window exists in which the shared resource can be modified by another code sequence that is operating concurrently.
CVSS v3 base score: 8.1
Published: 2019-04-22T16:29Z
Repository: https://github.com/lxc/lxd
Language: Go
Files changed: shared/idmapset_linux.go, shared/util_linux.go
Commit date: 2015-10-04 16:58:56 +0100
Commit message: CVE-2015-1340: Fix race condition between fchown and chmod in idmapset

Shifting a container filesystem in an environment where a user can
modify the container's filesystem as it's being shifted (for example if
the LXD server's / was shared over the network) would allow them to make
use of a race (TOCTOU) between the chown and chmod operations, allowing
for an arbitrary path to be chmod-ed rather than the planned container
path.

The fix is to use a file descriptor to the entry being processed,
validate that the entry itself is sane and then interact with the fd.

This is CVE-2015-1340

Reported-by: Seth Arnold
Signed-off-by: Stéphane Graber <stgraber@ubuntu.com>

Diff: diff --git a/shared/idmapset.go b/shared/idmapset_linux.go
similarity index 97%
rename from shared/idmapset.go
rename to shared/idmapset_linux.go
index 20050ef..d9bdbad 100644
--- a/shared/idmapset.go
+++ b/shared/idmapset_linux.go
@@ -221,10 +221,12 @@ func GetOwner(path string) (int, int, error) {
 	uid, gid, _, _, _, _, err := GetFileStat(path)
 	return uid, gid, err
 }
 
 func (set *IdmapSet) doUidshiftIntoContainer(dir string, testmode bool, how string) error {
+	dir = strings.TrimRight(dir, "/")
+
 	convert := func(path string, fi os.FileInfo, err error) (e error) {
 		uid, gid, err := GetOwner(path)
 		if err != nil {
 			return err
 		}
@@ -236,19 +238,13 @@ func (set *IdmapSet) doUidshiftIntoContainer(dir string, testmode bool, how stri
 			newuid, newgid = set.ShiftFromNs(uid, gid)
 		}
 		if testmode {
 			fmt.Printf("I would shift %q to %d %d\n", path, newuid, newgid)
 		} else {
-			err = os.Lchown(path, int(newuid), int(newgid))
-			if err == nil {
-				m := fi.Mode()
-				if m&os.ModeSymlink == 0 {
-					err = os.Chmod(path, m)
-					if err != nil {
-						fmt.Printf("Error resetting mode on %q, continuing\n", path)
-					}
-				}
+			err = ShiftOwner(dir, path, int(newuid), int(newgid))
+			if err != nil {
+				return err
 			}
 		}
 		return nil
 	}
 
diff --git a/shared/util_linux.go b/shared/util_linux.go
index 099b236..ead9c63 100644
--- a/shared/util_linux.go
+++ b/shared/util_linux.go
@@ -15,19 +15,28 @@ import (
 
 // #cgo LDFLAGS: -lutil -lpthread
 /*
 #define _GNU_SOURCE
 #include <sys/types.h>
+#include <sys/stat.h>
 #include <unistd.h>
 #include <stdlib.h>
 #include <grp.h>
 #include <pty.h>
 #include <errno.h>
 #include <fcntl.h>
+#include <limits.h>
 #include <string.h>
 #include <stdio.h>
-#include <stdio.h>
+
+#ifndef AT_SYMLINK_FOLLOW
+#define AT_SYMLINK_FOLLOW    0x400
+#endif
+
+#ifndef AT_EMPTY_PATH
+#define AT_EMPTY_PATH       0x1000
+#endif
 
 // This is an adaption from https://codereview.appspot.com/4589049, to be
 // included in the stdlib with the stdlib's license.
 
 static int mygetgrgid_r(int gid, struct group *grp,
@@ -105,13 +114,86 @@ void create_pipe(int *master, int *slave) {
 	}
 
 	*master = pipefd[0];
 	*slave = pipefd[1];
 }
+
+int shiftowner(char *basepath, char *path, int uid, int gid) {
+	struct stat sb;
+	int fd, r;
+	char fdpath[PATH_MAX];
+	char realpath[PATH_MAX];
+
+	fd = open(path, O_PATH|O_NOFOLLOW);
+	if (fd < 0 ) {
+		perror("Failed open");
+		return 1;
+	}
+
+	r = sprintf(fdpath, "/proc/self/fd/%d", fd);
+	if (r < 0) {
+		perror("Failed sprintf");
+		close(fd);
+		return 1;
+	}
+
+	r = readlink(fdpath, realpath, PATH_MAX);
+	if (r < 0) {
+		perror("Failed readlink");
+		close(fd);
+		return 1;
+	}
+
+	if (strlen(realpath) < strlen(basepath)) {
+		printf("Invalid path, source is outside of basepath.\n");
+		close(fd);
+		return 1;
+	}
+
+	if (strncmp(realpath, basepath, strlen(basepath))) {
+		printf("Invalid path, source is outside of basepath.\n");
+		close(fd);
+		return 1;
+	}
+
+	r = fstat(fd, &sb);
+	if (r < 0) {
+		perror("Failed fstat");
+		close(fd);
+		return 1;
+	}
+
+	r = fchownat(fd, "", uid, gid, AT_EMPTY_PATH|AT_SYMLINK_NOFOLLOW);
+	if (r < 0) {
+		perror("Failed chown");
+		close(fd);
+		return 1;
+	}
+
+	if (!S_ISLNK(sb.st_mode)) {
+		r = chmod(fdpath, sb.st_mode);
+		if (r < 0) {
+			perror("Failed chmod");
+			close(fd);
+			return 1;
+		}
+	}
+
+	close(fd);
+	return 0;
+}
 */
 import "C"
 
+func ShiftOwner(basepath string, path string, uid int, gid int) error {
+	r := C.shiftowner(C.CString(basepath), C.CString(path), C.int(uid), C.int(gid))
+	if r != 0 {
+		return fmt.Errorf("Failed to change ownership of: %s", path)
+	}
+	return nil
+}
+
 func OpenPty(uid, gid int) (master *os.File, slave *os.File, err error) {
 	fd_master := C.int(-1)
 	fd_slave := C.int(-1)
 	rootUid := C.int(uid)
 	rootGid := C.int(gid)

Vulnerable code: 			err = os.Lchown(path, int(newuid), int(newgid))
			if err == nil {
				m := fi.Mode()
				if m&os.ModeSymlink == 0 {
					err = os.Chmod(path, m)
					if err != nil {
						fmt.Printf("Error resetting mode on %q, continuing\n", path)
					}
				}
#include <stdio.h>
Fixed code: 	dir = strings.TrimRight(dir, "/")

			err = ShiftOwner(dir, path, int(newuid), int(newgid))
			if err != nil {
				return err
#include <sys/stat.h>
#include <limits.h>

#ifndef AT_SYMLINK_FOLLOW
#define AT_SYMLINK_FOLLOW    0x400
#endif

#ifndef AT_EMPTY_PATH
#define AT_EMPTY_PATH       0x1000
#endif

int shiftowner(char *basepath, char *path, int uid, int gid) {
	struct stat sb;
	int fd, r;
	char fdpath[PATH_MAX];
	char realpath[PATH_MAX];

	fd = open(path, O_PATH|O_NOFOLLOW);
	if (fd < 0 ) {
		perror("Failed open");
		return 1;
	}

	r = sprintf(fdpath, "/proc/self/fd/%d", fd);
	if (r < 0) {
		perror("Failed sprintf");
		close(fd);
		return 1;
	}

	r = readlink(fdpath, realpath, PATH_MAX);
	if (r < 0) {
		perror("Failed readlink");
		close(fd);
		return 1;
	}

	if (strlen(realpath) < strlen(basepath)) {
		printf("Invalid path, source is outside of basepath.\n");
		close(fd);
		return 1;
	}

	if (strncmp(realpath, basepath, strlen(basepath))) {
		printf("Invalid path, source is outside of basepath.\n");
		close(fd);
		return 1;
	}

	r = fstat(fd, &sb);
	if (r < 0) {
		perror("Failed fstat");
		close(fd);
		return 1;
	}

	r = fchownat(fd, "", uid, gid, AT_EMPTY_PATH|AT_SYMLINK_NOFOLLOW);
	if (r < 0) {
		perror("Failed chown");
		close(fd);
		return 1;
	}

	if (!S_ISLNK(sb.st_mode)) {
		r = chmod(fdpath, sb.st_mode);
		if (r < 0) {
			perror("Failed chmod");
			close(fd);
			return 1;
		}
	}

	close(fd);
	return 0;
}
func ShiftOwner(basepath string, path string, uid int, gid int) error {
	r := C.shiftowner(C.CString(basepath), C.CString(path), C.int(uid), C.int(gid))
	if r != 0 {
		return fmt.Errorf("Failed to change ownership of: %s", path)
	}
	return nil
}