Permalink
Browse files

If the server instance directory already exists, it is now removed fi…

…rst 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.
  • Loading branch information...
1 parent 5db9246 commit 5483b3292cc2af1c83033eaaadec20dba4dcfd9b @FooBarWidget FooBarWidget committed Jun 27, 2013
Showing with 35 additions and 2 deletions.
  1. +4 −0 NEWS
  2. +28 −1 ext/common/ServerInstanceDir.h
  3. +3 −1 test/cxx/ServerInstanceDirTest.cpp
View
4 NEWS
@@ -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.
@@ -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 {
@@ -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);

0 comments on commit 5483b32

Please sign in to comment.