Fix low-urgency security vulnerability: writing files to arbitrary di…

…rectory by hijacking temp directories.

NEWS

@@ -17,6 +17,36 @@ Release 4.0.37
    using this command guarantees that clients see no errors.
  * Fixed a crash occurs when an application fails to spawn, but the HTTP
    client disconnects before the error page is generated. Fixes issue #1028.
+ * Fixed a symlink-related security vulnerability.
+
+   Urgency: low
+   Scope: local exploit
+   Summary: writing files to arbitrary directory by hijacking temp directories
+   Affected versions: 4.0.5 and later
+   Fixed versions: 4.0.37
+
+   Description:
+   Phusion Passenger creates a "server instance directory" in /tmp during startup,
+   which is a temporary directory that Phusion Passenger uses to store working files.
+   This directory is deleted after Phusion Passenger exits. For various technical
+   reasons, this directory must have a semi-predictable filename. If a local attacker
+   can predict this filename, and precreates a symlink with the same filename that
+   points to an arbitrary directory with mode 755, owner root and group root, then
+   the attacker will succeed in making Phusion Passenger write files and create
+   subdirectories inside that target directory. The following files/subdirectories
+   are created:
+
+    * control_process.pid
+    * generation-X, where X is a number.
+
+   If you happen to have a file inside the target directory called `control_process.pid`,
+   then that file's contents are overwritten.
+
+   These files and directories are deleted during Phusion Passenger exit. The target
+   directory itself is not deleted, nor are any other contents inside the target
+   directory, although the symlink is.
+
+   Thanks go to Jakub Wilk for discovering this issue.
 
 
 Release 4.0.36

ext/common/ServerInstanceDir.h

@@ -213,7 +213,7 @@ class ServerInstanceDir: public noncopyable {
 		 * generations no matter what user they're running as.
 		 */
 		if (owner) {
-			switch (getFileType(path)) {
+			switch (getFileTypeNoFollowSymlinks(path)) {
 			case FT_NONEXISTANT:
 				createDirectory(path);
 				break;

ext/common/Utils.cpp

@@ -143,6 +143,35 @@ getFileType(const StaticString &filename, CachedFileStat *cstat, unsigned int th
 	}
 }
 
+FileType
+getFileTypeNoFollowSymlinks(const StaticString &filename) {
+	struct stat buf;
+	int ret;
+
+	ret = lstat(filename.c_str(), &buf);
+	if (ret == 0) {
+		if (S_ISREG(buf.st_mode)) {
+			return FT_REGULAR;
+		} else if (S_ISDIR(buf.st_mode)) {
+			return FT_DIRECTORY;
+		} else if (S_ISLNK(buf.st_mode)) {
+			return FT_SYMLINK;
+		} else {
+			return FT_OTHER;
+		}
+	} else {
+		if (errno == ENOENT) {
+			return FT_NONEXISTANT;
+		} else {
+			int e = errno;
+			string message("Cannot lstat '");
+			message.append(filename);
+			message.append("'");
+			throw FileSystemException(message, e, filename);
+		}
+	}
+}
+
 void
 createFile(const string &filename, const StaticString &contents, mode_t permissions, uid_t owner,
 	gid_t group, bool overwrite)

ext/common/Utils.h

@@ -65,6 +65,8 @@ typedef enum {
 	FT_REGULAR,
 	/** A directory. */
 	FT_DIRECTORY,
+	/** A symlink. Only returned by getFileTypeNoFollowSymlinks(), not by getFileType(). */
+	FT_SYMLINK,
 	/** Something else, e.g. a pipe or a socket. */
 	FT_OTHER
 } FileType;
@@ -110,7 +112,7 @@ bool fileExists(const StaticString &filename, CachedFileStat *cstat = 0,
 /**
  * Check whether 'filename' exists and what kind of file it is.
  *
- * @param filename The filename to check.
+ * @param filename The filename to check. It MUST be NULL-terminated.
  * @param mstat A CachedFileStat object, if you want to use cached statting.
  * @param throttleRate A throttle rate for cstat. Only applicable if cstat is not NULL.
  * @return The file type.
@@ -121,6 +123,10 @@ bool fileExists(const StaticString &filename, CachedFileStat *cstat = 0,
  */
 FileType getFileType(const StaticString &filename, CachedFileStat *cstat = 0,
                      unsigned int throttleRate = 0);
+/**
+ * Like getFileType(), but does not follow symlinks.
+ */
+FileType getFileTypeNoFollowSymlinks(const StaticString &filename);
 
 /**
  * Create the given file with the given contents, permissions and ownership.