[dovecot-cvs] dovecot/src/lib unlink-directory.c,1.5,1.6

[cras at procontrol.fi]

Update of /home/cvs/dovecot/src/lib
In directory danu:/tmp/cvs-serv24622

Modified Files:
	unlink-directory.c 
Log Message:
unlink_directory() is now (hopefully) race-condition free.



Index: unlink-directory.c
===================================================================
RCS file: /home/cvs/dovecot/src/lib/unlink-directory.c,v
retrieving revision 1.5
retrieving revision 1.6
diff -u -d -r1.5 -r1.6
--- unlink-directory.c	21 Dec 2002 12:13:58 -0000	1.5
+++ unlink-directory.c	24 Feb 2003 01:16:30 -0000	1.6
@@ -1,7 +1,7 @@
 /*
- unlink-directory.c : Unlink directory with everything under it.
+ unlink-directory.c : Safely unlink directory with everything under it.
 
-    Copyright (c) 2002 Timo Sirainen
+    Copyright (c) 2002-2003 Timo Sirainen
 
     Permission is hereby granted, free of charge, to any person obtaining
     a copy of this software and associated documentation files (the
@@ -23,24 +23,104 @@
     SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
 */
 
+#define _GNU_SOURCE /* for O_NOFOLLOW with Linux */
+
 #include "lib.h"
 #include "unlink-directory.h"
 
+#include <fcntl.h>
 #include <unistd.h>
 #include <dirent.h>
 #include <sys/stat.h>
 
-int unlink_directory(const char *dir, int unlink_dir)
+#define close_save_errno(fd) \
+	STMT_START { \
+		old_errno = errno; \
+		(void)close(fd); \
+		errno = old_errno; \
+	} STMT_END
+
+static int unlink_directory_r(const char *dir)
 {
 	DIR *dirp;
 	struct dirent *d;
 	struct stat st;
-	char path[PATH_MAX];
+        int dir_fd, old_errno;
 
-	dirp = opendir(dir);
-	if (dirp == NULL)
-		return errno == ENOENT ? 0 : -1;
+	/* There's a bit tricky race condition with recursive deletion.
+	   Suppose this happens:
+
+	   lstat(dir, ..) -> OK, it's a directory
+	   // attacker deletes dir, replaces it with symlink to /
+	   opendir(dir) -> it actually opens /
 
+	   Most portable solution is to lstat() the dir, chdir() there, then
+	   check that "." points to same device/inode as we originally
+	   lstat()ed. This assumes that the device has usable inodes, most
+	   should except for some NFS implementations.
+
+	   Filesystems may also reassign a deleted inode to another file
+	   immediately after it's deleted. That in theory makes it possible to
+	   exploit this race to delete the new directory. However, the new
+	   inode is quite unlikely to be any important directory, and attacker
+	   is quite unlikely to find out which directory even got the inode.
+	   Maybe with some setuid program or daemon interaction something could
+	   come out of it though.
+
+	   Another less portable solution is to fchdir(open(dir, O_NOFOLLOW)).
+	   This should be completely safe.
+
+	   The actual file deletion also has to be done relative to current
+	   directory, to make sure that the whole directory structure isn't
+	   replaced with another one while we're deleting it. Going back to
+	   parent directory isn't too easy either - safest (and easiest) way
+	   again is to open() the directory and fchdir() back there.
+	*/
+
+#ifdef O_NOFOLLOW
+	dir_fd = open(dir, O_RDONLY | O_NOFOLLOW);
+	if (dir_fd == -1)
+		return -1;
+#else
+	struct stat st2;
+
+	if (lstat(dir, &st) < 0)
+		return -1;
+
+	if (!S_ISDIR(st.st_mode)) {
+		errno = ENOTDIR;
+		return -1;
+	}
+
+	dir_fd = open(dir, O_RDONLY);
+	if (dir_fd == -1)
+		return -1;
+
+	if (fstat(dir_fd, &st2) < 0) {
+		close_save_errno(dir_fd);
+		return -1;
+	}
+
+	if (st.st_ino != st2.st_ino ||
+	    !CMP_DEV_T(st.st_dev, st2.st_dev)) {
+		/* directory was just replaced with something else. */
+		(void)close(dir_fd);
+		errno = ENOTDIR;
+		return -1;
+	}
+#endif
+	if (fchdir(dir_fd) < 0) {
+                close_save_errno(dir_fd);
+		return -1;
+	}
+
+	dirp = opendir(".");
+	if (dirp == NULL) {
+		close_save_errno(dir_fd);
+		return -1;
+	}
+
+	errno = 0;
 	while ((d = readdir(dirp)) != NULL) {
 		if (d->d_name[0] == '.' &&
 		    (d->d_name[1] == '\0' ||
@@ -49,31 +129,66 @@
 			continue;
 		}
 
-		if (str_path(path, sizeof(path), dir, d->d_name) < 0)
-			return -1;
+		if (unlink(d->d_name) == -1 && errno != ENOENT) {
+			old_errno = errno;
 
-		if (unlink(path) == -1 && errno != ENOENT) {
-			int old_errno = errno;
+			if (lstat(d->d_name, &st) == 0 && S_ISDIR(st.st_mode)) {
+				if (unlink_directory_r(d->d_name) < 0) {
+					if (errno != ENOENT)
+						break;
+					errno = 0;
+				}
+				if (fchdir(dir_fd) < 0)
+					break;
 
-			if (lstat(path, &st) == 0 && S_ISDIR(st.st_mode)) {
-				if (unlink_directory(path, TRUE) < 0)
-					return -1;
+				if (rmdir(d->d_name) < 0) {
+					if (errno != ENOENT)
+						break;
+					errno = 0;
+				}
 			} else {
-				/* so it wasn't a directory, unlink() again
-				   to get correct errno */
+				/* so it wasn't a directory */
 				errno = old_errno;
-				return -1;
+				break;
 			}
 		}
 	}
 
+	(void)close(dir_fd);
+
+	old_errno = errno;
 	if (closedir(dirp) < 0)
 		return -1;
 
+	if (old_errno != 0) {
+		errno = old_errno;
+		return -1;
+	}
+
+	return 0;
+}
+
+int unlink_directory(const char *dir, int unlink_dir)
+{
+	int fd, ret;
+
+	fd = open(".", O_RDONLY);
+	if (fd == -1)
+		return -1;
+
+	ret = unlink_directory_r(dir);
+	if (ret < 0 && errno == ENOENT)
+		ret = 0;
+
+	if (fchdir(fd) < 0) {
+		i_fatal("unlink_directory(%s): "
+			"Can't fchdir() back to our original dir: %m", dir);
+	}
+
 	if (unlink_dir) {
-		if (rmdir(dir) == -1 && errno != ENOENT)
+		if (rmdir(dir) < 0 && errno != ENOENT)
 			return -1;
 	}
 
-	return 0;
+	return ret;
 }