From 15131cc4552b38de0638fa84164e509b05bb6c11 Mon Sep 17 00:00:00 2001 From: Emmanuel Lacour Date: Wed, 4 Jun 2008 08:36:42 +0000 Subject: [PATCH] r9183@datura: manu | 2008-06-04 09:59:07 +0200 Incorporate NMU (0.6.2-2.1) = revert CVE patch and make it a dpatch --- debian/changelog | 10 +- debian/patches/00list | 1 + debian/patches/04_CVE-2008-1614.dpatch | 302 +++++++++++++++++++++++++++++++++ src/API.hpp | 6 - src/API_Linux.cpp | 17 +- src/API_Linux.hpp | 5 - src/Application.cpp | 88 +++------- src/Application.hpp | 8 - src/File.cpp | 7 - src/File.hpp | 6 +- 10 files changed, 339 insertions(+), 111 deletions(-) create mode 100644 debian/patches/04_CVE-2008-1614.dpatch diff --git a/debian/changelog b/debian/changelog index 9fb1483..35bd0c0 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,10 +1,10 @@ -suphp (0.6.2-2+lenny0) testing-security; urgency=high +suphp (0.6.2-2.1) unstable; urgency=high - * The following security issue is addressed with this update: - - CVE-2008-1614: allows local users to gain privileges via a race - condition in symlinks handling + * Non-maintainer upload by the security team + * Fix race condition in symlink handling by adding + 04_CVE-2008-1614.dpatch (Closes: #475431) Fixes: CVE-2008-1614 - -- Emmanuel Lacour Fri, 11 Apr 2008 11:31:28 +0200 + -- Steffen Joeris Sat, 10 May 2008 08:48:45 +0000 suphp (0.6.2-2) unstable; urgency=low diff --git a/debian/patches/00list b/debian/patches/00list index f4d65ff..4ac9c90 100644 --- a/debian/patches/00list +++ b/debian/patches/00list @@ -1,3 +1,4 @@ 01_debian.dpatch 02_rsrc_conf.dpatch 03_libtool.dpatch +04_CVE-2008-1614.dpatch diff --git a/debian/patches/04_CVE-2008-1614.dpatch b/debian/patches/04_CVE-2008-1614.dpatch new file mode 100644 index 0000000..e378821 --- /dev/null +++ b/debian/patches/04_CVE-2008-1614.dpatch @@ -0,0 +1,302 @@ +#! /bin/sh /usr/share/dpatch/dpatch-run +## 04_CVE-2008-1614.dpatch +## +## All lines beginning with `## DP:' are a description of the patch. +## DP: Libtool update + +@DPATCH@ +--- suphp-0.6.2.orig/src/API_Linux.hpp ++++ suphp-0.6.2/src/API_Linux.hpp +@@ -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) + */ +only in patch2: +unchanged: +--- suphp-0.6.2.orig/src/File.cpp ++++ suphp-0.6.2/src/File.cpp +@@ -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); ++} +only in patch2: +unchanged: +--- suphp-0.6.2.orig/src/Application.hpp ++++ suphp-0.6.2/src/Application.hpp +@@ -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: + /** +only in patch2: +unchanged: +--- suphp-0.6.2.orig/src/Application.cpp ++++ suphp-0.6.2/src/Application.cpp +@@ -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(); +only in patch2: +unchanged: +--- suphp-0.6.2.orig/src/API_Linux.cpp ++++ suphp-0.6.2/src/API_Linux.cpp +@@ -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) { +only in patch2: +unchanged: +--- suphp-0.6.2.orig/src/File.hpp ++++ suphp-0.6.2/src/File.hpp +@@ -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); + }; + }; + +only in patch2: +unchanged: +--- suphp-0.6.2.orig/src/API.hpp ++++ suphp-0.6.2/src/API.hpp +@@ -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 --git a/src/API.hpp b/src/API.hpp index c079888..47f71e3 100644 --- a/src/API.hpp +++ b/src/API.hpp @@ -157,12 +157,6 @@ namespace suPHP { 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 --git a/src/API_Linux.cpp b/src/API_Linux.cpp index a504603..97c1e90 100644 --- a/src/API_Linux.cpp +++ b/src/API_Linux.cpp @@ -225,10 +225,10 @@ std::string suPHP::API_Linux::GroupInfo_getGroupname(const GroupInfo& ginfo) bool suPHP::API_Linux::File_exists(const File& file) const { struct stat dummy; - if (::lstat(file.getPath().c_str(), &dummy) == 0) - return true; + if (::stat(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 @@ std::string suPHP::API_Linux::File_getRealPath(const File& file) const bool suPHP::API_Linux::File_hasPermissionBit(const File& file, FileMode perm) const throw (SystemException) { struct stat temp; - if (lstat(file.getPath().c_str(), &temp) == -1) { + if (stat(file.getPath().c_str(), &temp) == -1) { throw SystemException(std::string("Could not stat \"") + file.getPath() + "\": " + ::strerror(errno), __FILE__, __LINE__); @@ -362,7 +362,7 @@ bool suPHP::API_Linux::File_hasPermissionBit(const File& file, FileMode perm) UserInfo suPHP::API_Linux::File_getUser(const File& file) const throw (SystemException) { struct stat temp; - if (lstat(file.getPath().c_str(), &temp) == -1) { + if (stat(file.getPath().c_str(), &temp) == -1) { throw SystemException(std::string("Could not stat \"") + file.getPath() + "\": " + ::strerror(errno), __FILE__, __LINE__); @@ -373,7 +373,7 @@ UserInfo suPHP::API_Linux::File_getUser(const File& file) const GroupInfo suPHP::API_Linux::File_getGroup(const File& file) const throw (SystemException) { struct stat temp; - if (lstat(file.getPath().c_str(), &temp) == -1) { + if (stat(file.getPath().c_str(), &temp) == -1) { throw SystemException(std::string("Could not stat \"") + file.getPath() + "\": " + ::strerror(errno), __FILE__, __LINE__); @@ -382,11 +382,6 @@ GroupInfo suPHP::API_Linux::File_getGroup(const File& file) const } -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 --git a/src/API_Linux.hpp b/src/API_Linux.hpp index 83b07a4..a547d40 100644 --- a/src/API_Linux.hpp +++ b/src/API_Linux.hpp @@ -169,11 +169,6 @@ namespace suPHP { 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 --git a/src/Application.cpp b/src/Application.cpp index 72e6731..bb7ce50 100644 --- a/src/Application.cpp +++ b/src/Application.cpp @@ -177,7 +177,6 @@ void suPHP::Application::checkScriptFile( 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()) { @@ -185,13 +184,11 @@ void suPHP::Application::checkScriptFile( logger.logWarning(error); throw SoftException(error, __FILE__, __LINE__); } - 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__); - } + + // Get full path to script file + + File realScriptFile = File(scriptFile.getRealPath()); + File directory = realScriptFile.getParentDirectory(); // Check wheter script is in docroot if (realScriptFile.getPath().find(config.getDocroot()) != 0) { @@ -216,19 +213,8 @@ void suPHP::Application::checkScriptFile( 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 permissions - // Directories will be checked later + // Check script and directory permissions if (!realScriptFile.hasUserReadBit()) { std::string error = "File \"" + realScriptFile.getPath() + "\" not readable"; @@ -245,6 +231,14 @@ void suPHP::Application::checkScriptFile( 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() @@ -253,6 +247,14 @@ void suPHP::Application::checkScriptFile( 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()) { @@ -272,8 +274,7 @@ void suPHP::Application::changeProcessPermissions( UserInfo targetUser; GroupInfo targetGroup; - File scriptFile = File(scriptFilename); - File realScriptFile = File(scriptFile.getRealPath()); + File scriptFile = File(File(scriptFilename).getRealPath()); API& api = API_Helper::getSystemAPI(); Logger& logger = api.getSystemLogger(); @@ -359,11 +360,7 @@ void suPHP::Application::changeProcessPermissions( 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 @@ -483,43 +480,6 @@ void suPHP::Application::executeScript(const std::string& scriptFilename, } -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 --git a/src/Application.hpp b/src/Application.hpp index 0aed9c8..be7ed82 100644 --- a/src/Application.hpp +++ b/src/Application.hpp @@ -107,14 +107,6 @@ namespace suPHP { 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 --git a/src/File.cpp b/src/File.cpp index b0b20c2..6c6d303 100644 --- a/src/File.cpp +++ b/src/File.cpp @@ -57,9 +57,6 @@ std::string suPHP::File::getRealPath() const throw (SystemException) { File suPHP::File::getParentDirectory() const { std::string path = this->getPath(); path = path.substr(0, path.rfind('/')); - if (path.length() == 0) { - path = "/"; - } return File(path); } @@ -107,7 +104,3 @@ UserInfo suPHP::File::getUser() const throw (SystemException) { 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 --git a/src/File.hpp b/src/File.hpp index cf839e7..3268fb7 100644 --- a/src/File.hpp +++ b/src/File.hpp @@ -143,11 +143,7 @@ namespace suPHP { * Returns owning group of file */ GroupInfo getGroup() const throw (SystemException); - - /** - * Checks whether this file is a symlink - */ - bool isSymlink() const throw (SystemException); + }; }; -- 2.11.0