Attachment 299694 Details for Bug 439687 – Fixes symlink checks

[patch] Fixes symlink checks

suphp-symlink.patch (text/plain), 10.58 KB, created by Wouter de Jong on 2008-03-31 09:58:13 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Wouter de Jong

Created: 2008-03-31 09:58:13 UTC

Size: 10.58 KB

patch

obsolete

>diff -rNu suphp-0.6.2/src/API.hpp suphp-0.6.3/src/API.hpp
>--- suphp-0.6.2/src/API.hpp	2005-11-26 20:29:02.000000000 +0100
>+++ suphp-0.6.3/src/API.hpp	2008-03-30 13:43:38.000000000 +0200
>@@ -157,6 +157,12 @@
> 	virtual GroupInfo File_getGroup(const File& file) const
> 	    throw (SystemException) =0;
> 	
>+        /**
>+         * Checks whether a file is a symlink
>+         */
>+        virtual bool File_isSymlink(const File& file) const 
>+            throw (SystemException) =0;
>+        
> 	/**
> 	 * Runs another program (replaces current process)
> 	 */
>diff -rNu suphp-0.6.2/src/API_Linux.cpp suphp-0.6.3/src/API_Linux.cpp
>--- suphp-0.6.2/src/API_Linux.cpp	2006-09-25 16:09:45.000000000 +0200
>+++ suphp-0.6.3/src/API_Linux.cpp	2008-03-30 13:43:38.000000000 +0200
>@@ -225,10 +225,10 @@
> 
> bool suPHP::API_Linux::File_exists(const File& file) const {
>     struct stat dummy;
>-    if (::stat(file.getPath().c_str(), &dummy) == 0)
>-	return true;
>+    if (::lstat(file.getPath().c_str(), &dummy) == 0)
>+        return true;
>     else
>-	return false;
>+        return false;
> }
> 
> std::string suPHP::API_Linux::File_getRealPath(const File& file) const
>@@ -304,7 +304,7 @@
> bool suPHP::API_Linux::File_hasPermissionBit(const File& file, FileMode perm) 
>     const throw (SystemException) {
>     struct stat temp;
>-    if (stat(file.getPath().c_str(), &temp) == -1) {
>+    if (lstat(file.getPath().c_str(), &temp) == -1) {
> 	throw SystemException(std::string("Could not stat \"")
> 			      + file.getPath() + "\": "
> 			      + ::strerror(errno), __FILE__, __LINE__);
>@@ -362,7 +362,7 @@
> UserInfo suPHP::API_Linux::File_getUser(const File& file) const
>     throw (SystemException) {
>     struct stat temp;
>-    if (stat(file.getPath().c_str(), &temp) == -1) {
>+    if (lstat(file.getPath().c_str(), &temp) == -1) {
> 	throw SystemException(std::string("Could not stat \"")
> 			      + file.getPath() + "\": "
> 			      + ::strerror(errno), __FILE__, __LINE__);
>@@ -373,7 +373,7 @@
> GroupInfo suPHP::API_Linux::File_getGroup(const File& file) const
>     throw (SystemException) {
>     struct stat temp;
>-    if (stat(file.getPath().c_str(), &temp) == -1) {
>+    if (lstat(file.getPath().c_str(), &temp) == -1) {
> 	throw SystemException(std::string("Could not stat \"")
> 			      + file.getPath() + "\": "
> 			      + ::strerror(errno), __FILE__, __LINE__);
>@@ -382,6 +382,11 @@
> }
> 
> 
>+bool suPHP::API_Linux::File_isSymlink(const File& file) const throw (SystemException) {
>+    return this->isSymlink(file.getPath());
>+}
>+
>+
> void suPHP::API_Linux::execute(std::string program, const CommandLine& cline,
> 			       const Environment& env) const
>     throw (SystemException) {
>diff -rNu suphp-0.6.2/src/API_Linux.hpp suphp-0.6.3/src/API_Linux.hpp
>--- suphp-0.6.2/src/API_Linux.hpp	2005-11-26 20:29:02.000000000 +0100
>+++ suphp-0.6.3/src/API_Linux.hpp	2008-03-30 13:43:38.000000000 +0200
>@@ -169,6 +169,11 @@
> 	virtual GroupInfo File_getGroup(const File& file) const
> 	    throw (SystemException);
> 
>+        /**
>+         * Checks whether a file is a symlink
>+         */
>+        virtual bool File_isSymlink(const File& file) const throw (SystemException);
>+        
> 	/**
> 	 * Runs another program (replaces current process)
> 	 */
>diff -rNu suphp-0.6.2/src/Application.cpp suphp-0.6.3/src/Application.cpp
>--- suphp-0.6.2/src/Application.cpp	2006-02-05 21:21:03.000000000 +0100
>+++ suphp-0.6.3/src/Application.cpp	2008-03-30 13:43:38.000000000 +0200
>@@ -177,6 +177,7 @@
>     throw (SystemException, SoftException) {
>     Logger& logger = API_Helper::getSystemAPI().getSystemLogger();
>     File scriptFile = File(scriptFilename);
>+    File realScriptFile = File(scriptFile.getRealPath());
> 
>     // Check wheter file exists
>     if (!scriptFile.exists()) {
>@@ -184,11 +185,13 @@
> 	logger.logWarning(error);
> 	throw SoftException(error, __FILE__, __LINE__);
>     }
>-    
>-    // Get full path to script file
>-
>-    File realScriptFile = File(scriptFile.getRealPath());
>-    File directory = realScriptFile.getParentDirectory();
>+    if (!realScriptFile.exists()) {
>+        std::string error = "File " + realScriptFile.getPath() 
>+            + " referenced by symlink " +scriptFile.getPath() 
>+            + " does not exist";
>+        logger.logWarning(error);
>+        throw SoftException(error, __FILE__, __LINE__);
>+    }
>     
>     // Check wheter script is in docroot
>     if (realScriptFile.getPath().find(config.getDocroot()) != 0) {
>@@ -213,8 +216,19 @@
> 	logger.logWarning(error);
> 	throw SoftException(error, __FILE__, __LINE__);
>     }
>+    if (config.getCheckVHostDocroot()
>+        && scriptFile.getPath().find(environment.getVar("DOCUMENT_ROOT")) 
>+        != 0) {
>+        
>+        std::string error = "File \"" + scriptFile.getPath()
>+            + "\" is not in document root of Vhost \""
>+            + environment.getVar("DOCUMENT_ROOT") + "\"";
>+        logger.logWarning(error);
>+        throw SoftException(error, __FILE__, __LINE__);
>+    }
> 
>-    // Check script and directory permissions
>+    // Check script permissions
>+    // Directories will be checked later
>     if (!realScriptFile.hasUserReadBit()) {
> 	std::string error = "File \"" + realScriptFile.getPath()
> 	    + "\" not readable";
>@@ -231,14 +245,6 @@
> 	throw SoftException(error, __FILE__, __LINE__);
>     }
>     
>-    if (!config.getAllowDirectoryGroupWriteable() 
>-	&& directory.hasGroupWriteBit()) {
>-	std::string error = "Directory \"" + directory.getPath()
>-	    + "\" is writeable by group";
>-	logger.logWarning(error);
>-	throw SoftException(error, __FILE__, __LINE__);
>-    }
>-
>     if (!config.getAllowFileOthersWriteable()
> 	&& realScriptFile.hasOthersWriteBit()) {
> 	std::string error = "File \"" + realScriptFile.getPath()
>@@ -247,14 +253,6 @@
> 	throw SoftException(error, __FILE__, __LINE__);
>     }
>     
>-    if (!config.getAllowDirectoryOthersWriteable()
>-	&& directory.hasOthersWriteBit()) {
>-	std::string error = "Directory \"" + directory.getPath()
>-	    + "\" is writeable by others";
>-	logger.logWarning(error);
>-	throw SoftException(error, __FILE__, __LINE__);
>-    }
>-
>     // Check UID/GID of symlink is matching target
>     if (scriptFile.getUser() != realScriptFile.getUser()
> 	|| scriptFile.getGroup() != realScriptFile.getGroup()) {
>@@ -274,7 +272,8 @@
>     UserInfo targetUser;
>     GroupInfo targetGroup;
> 
>-    File scriptFile = File(File(scriptFilename).getRealPath());
>+    File scriptFile = File(scriptFilename);
>+    File realScriptFile = File(scriptFile.getRealPath());
>     API& api = API_Helper::getSystemAPI();
>     Logger& logger = api.getSystemLogger();
> 
>@@ -360,7 +359,11 @@
> 	throw SoftException(error, __FILE__, __LINE__);
>     }
> #endif // OPT_USERGROUP_PARANOID    
>-
>+    
>+    // Check directory ownership and permissions
>+    checkParentDirectories(realScriptFile, targetUser, config);
>+    checkParentDirectories(scriptFile, targetUser, config);
>+    
>     // Common code used for all modes
> 
>     // Set new group first, because we still need super-user privileges
>@@ -480,6 +483,43 @@
> }
> 
> 
>+void suPHP::Application::checkParentDirectories(const File& file,
>+                                               const UserInfo& owner,
>+                                               const Configuration& config) const throw (SoftException) {
>+    File directory = file;
>+    Logger& logger = API_Helper::getSystemAPI().getSystemLogger();
>+    do {
>+        directory = directory.getParentDirectory();
>+        
>+        UserInfo directoryOwner = directory.getUser();
>+        if (directoryOwner != owner && !directoryOwner.isSuperUser()) {
>+            std::string error = "Directory " + directory.getPath()
>+                + " is not owned by " + owner.getUsername();
>+            logger.logWarning(error);
>+            throw SoftException(error, __FILE__, __LINE__);
>+        }
>+        
>+        if (!directory.isSymlink()
>+            && !config.getAllowDirectoryGroupWriteable() 
>+            && directory.hasGroupWriteBit()) {
>+            std::string error = "Directory \"" + directory.getPath()
>+                + "\" is writeable by group";
>+            logger.logWarning(error);
>+            throw SoftException(error, __FILE__, __LINE__);
>+        }
>+        
>+        if (!directory.isSymlink()
>+            && !config.getAllowDirectoryOthersWriteable()
>+            && directory.hasOthersWriteBit()) {
>+            std::string error = "Directory \"" + directory.getPath()
>+                + "\" is writeable by others";
>+            logger.logWarning(error);
>+            throw SoftException(error, __FILE__, __LINE__);
>+        }
>+    } while (directory.getPath() != "/");
>+}
>+
>+
> int main(int argc, char **argv) {
>     try {
> 	API& api = API_Helper::getSystemAPI();
>diff -rNu suphp-0.6.2/src/Application.hpp suphp-0.6.3/src/Application.hpp
>--- suphp-0.6.2/src/Application.hpp	2005-02-27 18:53:05.000000000 +0100
>+++ suphp-0.6.3/src/Application.hpp	2008-03-29 23:58:58.000000000 +0100
>@@ -107,6 +107,14 @@
> 			   const Configuration& config) const
> 	    throw (SoftException);
> 
>+         
>+         /**
>+          * Checks ownership and permissions for parent directories
>+          */
>+         void checkParentDirectories(const File& file,
>+                                     const UserInfo& owner,
>+                                     const Configuration& config) const
>+             throw (SoftException);
> 
>     public:
> 	/**
>diff -rNu suphp-0.6.2/src/File.cpp suphp-0.6.3/src/File.cpp
>--- suphp-0.6.2/src/File.cpp	2005-02-27 18:53:05.000000000 +0100
>+++ suphp-0.6.3/src/File.cpp	2008-03-30 13:43:38.000000000 +0200
>@@ -57,6 +57,9 @@
> File suPHP::File::getParentDirectory() const {
>     std::string path = this->getPath();
>     path = path.substr(0, path.rfind('/'));
>+    if (path.length() == 0) {
>+        path = "/";
>+    }
>     return File(path);
> }
> 
>@@ -104,3 +107,7 @@
> GroupInfo suPHP::File::getGroup() const throw (SystemException) {
>     return API_Helper::getSystemAPI().File_getGroup(*this);
> }
>+
>+bool suPHP::File::isSymlink() const throw (SystemException) {
>+    return API_Helper::getSystemAPI().File_isSymlink(*this);
>+}
>diff -rNu suphp-0.6.2/src/File.hpp suphp-0.6.3/src/File.hpp
>--- suphp-0.6.2/src/File.hpp	2005-02-27 18:53:05.000000000 +0100
>+++ suphp-0.6.3/src/File.hpp	2008-03-30 13:43:38.000000000 +0200
>@@ -143,7 +143,11 @@
> 	 * Returns owning group of file
> 	 */
> 	GroupInfo getGroup() const throw (SystemException);
>-	
>+
>+        /**
>+         * Checks whether this file is a symlink
>+         */
>+        bool isSymlink() const throw (SystemException);
>     };
> };

Actions: View | Diff

Attachments on bug 439687: 299694