CVE-2013-4136

@@ -30,6 +30,7 @@
 #include <oxt/backtrace.hpp>
 
 #include <sys/types.h>
+#include <sys/stat.h>
 #include <dirent.h>
 #include <unistd.h>
 #include <pwd.h>
@@ -214,7 +215,33 @@ class ServerInstanceDir: public noncopyable {
 		 * rights though, because we want admin tools to be able to list the available
 		 * generations no matter what user they're running as.
 		 */
-		makeDirTree(path, "u=rwx,g=rx,o=rx");
+		if (owner) {
+			switch (getFileType(path)) {
+			case FT_NONEXISTANT:
+				createDirectory(path);
+				break;
+			case FT_DIRECTORY:
+				removeDirTree(path);
+				createDirectory(path);
+				break;
+			default:
+				throw RuntimeException("'" + path + "' already exists, and is not a directory");
+			}
+		} else if (getFileType(path) != FT_DIRECTORY) {
+			throw RuntimeException("Server instance directory '" + path +
+				"' does not exist");
+		}
+	}
+
+	void createDirectory(const string &path) const {
+		// We do not use makeDirTree() here. If an attacker creates a directory
+		// just before we do, then we want to abort because we want the directory
+		// to have specific permissions.
+		if (mkdir(path.c_str(), parseModeString("u=rwx,g=rx,o=rx")) == -1) {
+			int e = errno;
+			throw FileSystemException("Cannot create server instance directory '" +
+				path + "'", e, path);
+		}
 	}
 	
 	bool isDirectory(const string &dir, struct dirent *entry) const {

@@ -34,6 +34,10 @@ Release 4.0.6
    created with the setuid bit, when it should have sticky bit (to prevent
    existing files from being deleted or renamed by a user that doesn't own the
    file). This has now been fixed.
+ * If the server instance directory already exists, it will now be removed
+   first in order get correct directory permissions. If the directory still
+   exists after removal, Phusion Passenger aborts to avoid writing to a
+   directory with unexpected permissions. Fixes issue #910.
  * The installer now checks whether the system has enough virtual memory, and
    prints a helpful warning if it doesn't.
  * Linux/AArch64 compatibility fixes. Patch contributed by Dirk Mueller.

@@ -58,9 +58,11 @@ namespace tut {
 	}
 	
 	TEST_METHOD(5) {
-		// The destructor doesnn't remove the server instance directory if it
+		// The destructor doesn't remove the server instance directory if it
 		// wasn't created with the ownership flag or if it's been detached.
 		string path, path2;
+		makeDirTree(parentDir + "/passenger-test.1234");
+		makeDirTree(parentDir + "/passenger-test.5678");
 		{
 			ServerInstanceDir dir(parentDir + "/passenger-test.1234", false);
 			ServerInstanceDir dir2(parentDir + "/passenger-test.5678", false);