[RFC PATCH 4/4] libselinux: restorecon: pin file to avoid TOCTOU issues

["Christian Göttsche" <cgzones@googlemail.com>]

Pin the file to operate on in restorecon_sb() to prevent symlink attacks
in between the label database lookup, the current context query and the
final context write.  Also don't use the file information from
fts_read(3), which might also be out of sync.

Due to querying file information twice, one in fts_read(3) needed for
the cross device check and one on the pinned file descriptor for the
database lookup, there is a slight slowdown:

    [current]
    Time (mean ± σ):     14.456 s ±  0.306 s    [User: 45.863 s, System: 4.463 s]
    Range (min … max):   14.275 s … 15.294 s    10 runs

    [changed]
    Time (mean ± σ):     15.843 s ±  0.045 s    [User: 46.274 s, System: 9.495 s]
    Range (min … max):   15.787 s … 15.916 s    10 runs

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
 libselinux/src/selinux_restorecon.c | 43 +++++++++++++++++------------
 1 file changed, 25 insertions(+), 18 deletions(-)

diff --git a/libselinux/src/selinux_restorecon.c b/libselinux/src/selinux_restorecon.c
index 42ef30cb..12b85101 100644
--- a/libselinux/src/selinux_restorecon.c
+++ b/libselinux/src/selinux_restorecon.c
@@ -621,13 +621,13 @@ out:
 	return rc;
 }
 
-static int restorecon_sb(const char *pathname, const struct stat *sb,
-			    struct rest_flags *flags, bool first)
+static int restorecon_sb(const char *pathname, struct rest_flags *flags, bool first)
 {
 	char *newcon = NULL;
 	char *curcon = NULL;
 	char *newtypecon = NULL;
-	int rc;
+	int fd = -1, rc;
+	struct stat stat_buf;
 	bool updated = false;
 	const char *lookup_path = pathname;
 	float pc;
@@ -642,13 +642,21 @@ static int restorecon_sb(const char *pathname, const struct stat *sb,
 		lookup_path += rootpathlen;
 	}
 
+	fd = open(pathname, O_PATH | O_NOFOLLOW | O_EXCL);
+	if (fd < 0)
+		goto err;
+
+	rc = fstat(fd, &stat_buf);
+	if (rc < 0)
+		goto err;
+
 	if (rootpath != NULL && lookup_path[0] == '\0')
 		/* this is actually the root dir of the alt root. */
 		rc = selabel_lookup_raw(fc_sehandle, &newcon, "/",
-						    sb->st_mode);
+						    stat_buf.st_mode);
 	else
 		rc = selabel_lookup_raw(fc_sehandle, &newcon, lookup_path,
-						    sb->st_mode);
+						    stat_buf.st_mode);
 
 	if (rc < 0) {
 		if (errno == ENOENT) {
@@ -657,10 +665,10 @@ static int restorecon_sb(const char *pathname, const struct stat *sb,
 					    "Warning no default label for %s\n",
 					    lookup_path);
 
-			return 0; /* no match, but not an error */
+			goto out; /* no match, but not an error */
 		}
 
-		return -1;
+		goto err;
 	}
 
 	if (flags->progress) {
@@ -680,19 +688,17 @@ static int restorecon_sb(const char *pathname, const struct stat *sb,
 	}
 
 	if (flags->add_assoc) {
-		rc = filespec_add(sb->st_ino, newcon, pathname, flags);
+		rc = filespec_add(stat_buf.st_ino, newcon, pathname, flags);
 
 		if (rc < 0) {
 			selinux_log(SELINUX_ERROR,
 				    "filespec_add error: %s\n", pathname);
-			freecon(newcon);
-			return -1;
+			goto out1;
 		}
 
 		if (rc > 0) {
 			/* Already an association and it took precedence. */
-			freecon(newcon);
-			return 0;
+			goto out;
 		}
 	}
 
@@ -700,7 +706,7 @@ static int restorecon_sb(const char *pathname, const struct stat *sb,
 		selinux_log(SELINUX_INFO, "%s matched by %s\n",
 			    pathname, newcon);
 
-	if (lgetfilecon_raw(pathname, &curcon) < 0) {
+	if (fgetfilecon_raw(fd, &curcon) < 0) {
 		if (errno != ENODATA)
 			goto err;
 
@@ -733,7 +739,7 @@ static int restorecon_sb(const char *pathname, const struct stat *sb,
 		}
 
 		if (!flags->nochange) {
-			if (lsetfilecon(pathname, newcon) < 0)
+			if (fsetfilecon(fd, newcon) < 0)
 				goto err;
 			updated = true;
 		}
@@ -758,6 +764,8 @@ static int restorecon_sb(const char *pathname, const struct stat *sb,
 out:
 	rc = 0;
 out1:
+	if (fd >= 0)
+		close(fd);
 	freecon(curcon);
 	freecon(newcon);
 	return rc;
@@ -855,7 +863,6 @@ static void *selinux_restorecon_thread(void *arg)
 	FTSENT *ftsent;
 	int error;
 	char ent_path[PATH_MAX];
-	struct stat ent_st;
 	bool first = false;
 
 	if (state->parallel)
@@ -953,11 +960,11 @@ loop_body:
 			/* fall through */
 		default:
 			strcpy(ent_path, ftsent->fts_path);
-			ent_st = *ftsent->fts_statp;
+
 			if (state->parallel)
 				pthread_mutex_unlock(&state->mutex);
 
-			error = restorecon_sb(ent_path, &ent_st, &state->flags,
+			error = restorecon_sb(ent_path, &state->flags,
 					      first);
 
 			if (state->parallel) {
@@ -1153,7 +1160,7 @@ static int selinux_restorecon_common(const char *pathname_orig,
 			goto cleanup;
 		}
 
-		error = restorecon_sb(pathname, &sb, &state.flags, true);
+		error = restorecon_sb(pathname, &state.flags, true);
 		goto cleanup;
 	}
 
-- 
2.36.1