CVE-2014-1832: Other (NVD-CWE-Other)
value: Phusion Passenger 4.0.37 allows local users to write to certain files and directories via a symlink attack on (1) control_process.pid or a (2) generation-* file.  NOTE: this vulnerability exists because of an incomplete fix for CVE-2014-1831.
CWE description: NVD is only using a subset of CWE for mapping instead of the entire CWE, and the weakness type is not covered by that subset.
Published: 2015-02-19T15:59Z
Repository: https://github.com/phusion/passenger
Language: C
Files changed: NEWS, ext/common/ServerInstanceDir.h, ext/common/Utils.cpp, ext/common/Utils.h
Commit date: 2014-01-29 14:24:26 +0100
Commit message: Fix a symlink-related security vulnerability.

The fix in commit 34b10878 and contained a small attack time window in
between two filesystem operations. This has been fixed.

Diff: diff --git a/NEWS b/NEWS
index 98e37970d..178917001 100644
--- a/NEWS
+++ b/NEWS
@@ -1,5 +1,23 @@
+Release 4.0.38
+--------------
+
+ * 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.37
+   Fixed versions: 4.0.38
+
+   Description:
+   This issue is related to the security issue as mentioned in the 4.0.37
+   release notes. The previous fix was incomplete, and still has a
+   (albeit smaller) small attack time window in between two filesystem
+   checks. This attack window is now gone.
+
+
 Release 4.0.37
 --------------
 
  * Improved Node.js compatibility. Calling on() on the request object
    now returns the request object itself. This fixes some issues with
diff --git a/ext/common/ServerInstanceDir.h b/ext/common/ServerInstanceDir.h
index 8da3cf341..1315de557 100644
--- a/ext/common/ServerInstanceDir.h
+++ b/ext/common/ServerInstanceDir.h
@@ -1,8 +1,8 @@
 /*
  *  Phusion Passenger - https://www.phusionpassenger.com/
- *  Copyright (c) 2010-2013 Phusion
+ *  Copyright (c) 2010-2014 Phusion
  *
  *  "Phusion Passenger" is a trademark of Hongli Lai & Ninh Bui.
  *
  *  Permission is hereby granted, free of charge, to any person obtaining a copy
  *  of this software and associated documentation files (the "Software"), to deal
@@ -191,10 +191,13 @@ private:
 	
 	friend class Generation;
 	
 	void initialize(const string &path, bool owner) {
 		TRACE_POINT();
+		struct stat buf;
+		int ret;
+
 		this->path  = path;
 		this->owner = owner;
 		
 		/* Create the server instance directory. We only need to write to this
 		 * directory for these reasons:
@@ -210,22 +213,29 @@ private:
 		 * Therefore, we make the directory only writable by the user the helper server
 		 * was running as before privilege is lowered. Everybody else has read and execute
 		 * rights though, because we want admin tools to be able to list the available
 		 * generations no matter what user they're running as.
 		 */
+
+		do {
+			ret = lstat(path.c_str(), &buf);
+		} while (ret == -1 && errno == EAGAIN);
 		if (owner) {
-			switch (getFileTypeNoFollowSymlinks(path)) {
-			case FT_NONEXISTANT:
+			if (ret == 0) {
+				if (S_ISDIR(buf.st_mode)) {
+					verifyDirectoryPermissions(path, buf);
+				} else {
+					throw RuntimeException("'" + path + "' already exists, and is not a directory");
+				}
+			} else if (errno == ENOENT) {
 				createDirectory(path);
-				break;
-			case FT_DIRECTORY:
-				verifyDirectoryPermissions(path);
-				break;
-			default:
-				throw RuntimeException("'" + path + "' already exists, and is not a directory");
+			} else {
+				int e = errno;
+				throw FileSystemException("Cannot lstat '" + path + "'",
+					e, path);
 			}
-		} else if (getFileType(path) != FT_DIRECTORY) {
+		} else if (!S_ISDIR(buf.st_mode)) {
 			throw RuntimeException("Server instance directory '" + path +
 				"' does not exist");
 		}
 	}
 
@@ -257,18 +267,14 @@ private:
 	/**
 	 * When reusing an existing server instance directory, check permissions
 	 * so that an attacker cannot pre-create a directory with too liberal
 	 * permissions.
 	 */
-	void verifyDirectoryPermissions(const string &path) {
+	void verifyDirectoryPermissions(const string &path, struct stat &buf) {
 		TRACE_POINT();
-		struct stat buf;
 
-		if (stat(path.c_str(), &buf) == -1) {
-			int e = errno;
-			throw FileSystemException("Cannot stat() " + path, e, path);
-		} else if (buf.st_mode != (S_IFDIR | parseModeString("u=rwx,g=rx,o=rx"))) {
+		if (buf.st_mode != (S_IFDIR | parseModeString("u=rwx,g=rx,o=rx"))) {
 			throw RuntimeException("Tried to reuse existing server instance directory " +
 				path + ", but it has wrong permissions");
 		} else if (buf.st_uid != geteuid() || buf.st_gid != getegid()) {
 			/* The server instance directory is always created by the Watchdog. Its UID/GID never
 			 * changes because:
diff --git a/ext/common/Utils.cpp b/ext/common/Utils.cpp
index d1db8d6d0..1f3dec599 100644
--- a/ext/common/Utils.cpp
+++ b/ext/common/Utils.cpp
@@ -141,39 +141,10 @@ getFileType(const StaticString &filename, CachedFileStat *cstat, unsigned int th
 			throw FileSystemException(message, e, filename);
 		}
 	}
 }
 
-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)
 {
 	FileDescriptor fd;
diff --git a/ext/common/Utils.h b/ext/common/Utils.h
index 5cfaf9210..a04e50770 100644
--- a/ext/common/Utils.h
+++ b/ext/common/Utils.h
@@ -63,12 +63,10 @@ typedef enum {
 	FT_NONEXISTANT,
 	/** A regular file or a symlink to a regular file. */
 	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;
 
 /**
@@ -121,14 +119,10 @@ bool fileExists(const StaticString &filename, CachedFileStat *cstat = 0,
  * @throws boost::thread_interrupted
  * @ingroup Support
  */
 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.
  * This function does not leave behind junk files: if the ownership cannot be set
  * or if not all data can be written then then the file will be deleted.

Vulnerable code:  *  Copyright (c) 2010-2013 Phusion
			switch (getFileTypeNoFollowSymlinks(path)) {
			case FT_NONEXISTANT:
				break;
			case FT_DIRECTORY:
				verifyDirectoryPermissions(path);
				break;
			default:
				throw RuntimeException("'" + path + "' already exists, and is not a directory");
		} else if (getFileType(path) != FT_DIRECTORY) {
	void verifyDirectoryPermissions(const string &path) {
		struct stat buf;
		if (stat(path.c_str(), &buf) == -1) {
			int e = errno;
			throw FileSystemException("Cannot stat() " + path, e, path);
		} else if (buf.st_mode != (S_IFDIR | parseModeString("u=rwx,g=rx,o=rx"))) {
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);
		}
	}
}

	/** A symlink. Only returned by getFileTypeNoFollowSymlinks(), not by getFileType(). */
	FT_SYMLINK,
/**
 * Like getFileType(), but does not follow symlinks.
 */
FileType getFileTypeNoFollowSymlinks(const StaticString &filename);
Fixed code: Release 4.0.38
--------------

 * 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.37
   Fixed versions: 4.0.38

   Description:
   This issue is related to the security issue as mentioned in the 4.0.37
   release notes. The previous fix was incomplete, and still has a
   (albeit smaller) small attack time window in between two filesystem
   checks. This attack window is now gone.


 *  Copyright (c) 2010-2014 Phusion
		struct stat buf;
		int ret;


		do {
			ret = lstat(path.c_str(), &buf);
		} while (ret == -1 && errno == EAGAIN);
			if (ret == 0) {
				if (S_ISDIR(buf.st_mode)) {
					verifyDirectoryPermissions(path, buf);
				} else {
					throw RuntimeException("'" + path + "' already exists, and is not a directory");
				}
			} else if (errno == ENOENT) {
			} else {
				int e = errno;
				throw FileSystemException("Cannot lstat '" + path + "'",
					e, path);
		} else if (!S_ISDIR(buf.st_mode)) {
	void verifyDirectoryPermissions(const string &path, struct stat &buf) {
		if (buf.st_mode != (S_IFDIR | parseModeString("u=rwx,g=rx,o=rx"))) {