[DRE-maint] Bug#717176: ruby-passenger: CVE-2013-4136: insecure tmp files usage
Felix Geyer
fgeyer at debian.org
Sat Jul 20 17:32:42 UTC 2013
I've prepared a debdiff that cherry-picks another upstream
commit to properly fix CVE-2013-2119 and backports the two
commits necessary to fix CVE-2013-4136.
Cheers,
Felix
-------------- next part --------------
diff -Nru ruby-passenger-3.0.13debian/debian/changelog ruby-passenger-3.0.13debian/debian/changelog
--- ruby-passenger-3.0.13debian/debian/changelog 2013-05-30 12:41:00.000000000 +0200
+++ ruby-passenger-3.0.13debian/debian/changelog 2013-07-20 19:22:25.000000000 +0200
@@ -1,3 +1,12 @@
+ruby-passenger (3.0.13debian-1.2) unstable; urgency=high
+
+ * Non-maintainer upload.
+ * Cherry-pick another commit to properly fix CVE-2013-2119.
+ * Fix CVE-2013-4136: insecure tmp files usage. (Closes: #717176)
+ - Add CVE-2013-4136.patch, backported from upstream.
+
+ -- Felix Geyer <fgeyer at debian.org> Sat, 20 Jul 2013 10:56:34 +0200
+
ruby-passenger (3.0.13debian-1.1) unstable; urgency=low
* Non-maintainer upload.
diff -Nru ruby-passenger-3.0.13debian/debian/patches/CVE-2013-2119.patch ruby-passenger-3.0.13debian/debian/patches/CVE-2013-2119.patch
--- ruby-passenger-3.0.13debian/debian/patches/CVE-2013-2119.patch 2013-05-30 09:31:09.000000000 +0200
+++ ruby-passenger-3.0.13debian/debian/patches/CVE-2013-2119.patch 2013-07-20 13:37:26.000000000 +0200
@@ -1,18 +1,8 @@
-From 0eaebb00f6b7327374069a7998064c68cc54e9f1 Mon Sep 17 00:00:00 2001
-From: "Hongli Lai (Phusion)" <hongli at phusion.nl>
-Date: Tue, 28 May 2013 22:30:53 +0200
-Subject: [PATCH] Ensure that temporary files and directories didn't already
- exist.
+Description: Fix for CVE-2013-2119: insecure tmp files usage
+Origin: upstream, https://github.com/phusion/passenger/commit/0eaebb00f6b7327374069a7998064c68cc54e9f1
+ and https://github.com/phusion/passenger/commit/56d9d39fb522e0967acbde0bcec1afc37313ceb4
+Bug-Debian: http://bugs.debian.org/710351
----
- bin/passenger-install-nginx-module | 7 ++---
- lib/phusion_passenger/dependencies.rb | 32 ++++++++--------------
- lib/phusion_passenger/standalone/command.rb | 9 +++---
- .../standalone/runtime_installer.rb | 7 ++---
- 4 files changed, 23 insertions(+), 32 deletions(-)
-
-diff --git a/bin/passenger-install-nginx-module b/bin/passenger-install-nginx-module
-index 629240c..450252c 100755
--- a/bin/passenger-install-nginx-module
+++ b/bin/passenger-install-nginx-module
@@ -27,6 +27,7 @@ $LOAD_PATH.unshift("#{passenger_root}/lib")
@@ -40,8 +30,6 @@
end
private
-diff --git a/lib/phusion_passenger/dependencies.rb b/lib/phusion_passenger/dependencies.rb
-index e37a212..685b37d 100644
--- a/lib/phusion_passenger/dependencies.rb
+++ b/lib/phusion_passenger/dependencies.rb
@@ -22,6 +22,7 @@
@@ -136,8 +124,6 @@
end
end
if RUBY_PLATFORM =~ /linux/
-diff --git a/lib/phusion_passenger/standalone/command.rb b/lib/phusion_passenger/standalone/command.rb
-index 8810427..b84909f 100644
--- a/lib/phusion_passenger/standalone/command.rb
+++ b/lib/phusion_passenger/standalone/command.rb
@@ -172,8 +172,11 @@ def determine_various_resource_locations(create_subdirs = true)
@@ -163,8 +149,6 @@
if @options[:socket_file]
ping_spec = [:unix, @options[:socket_file]]
else
-diff --git a/lib/phusion_passenger/standalone/runtime_installer.rb b/lib/phusion_passenger/standalone/runtime_installer.rb
-index 730f776..31b6fd8 100644
--- a/lib/phusion_passenger/standalone/runtime_installer.rb
+++ b/lib/phusion_passenger/standalone/runtime_installer.rb
@@ -23,6 +23,7 @@
@@ -194,6 +178,109 @@
@plugin.call_hook(:runtime_installer_cleanup) if @plugin
end
---
-1.8.1.6
-
+--- a/lib/phusion_passenger/platform_info.rb
++++ b/lib/phusion_passenger/platform_info.rb
+@@ -21,6 +21,8 @@
+ # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ # THE SOFTWARE.
+
++require 'tmpdir'
++
+ module PhusionPassenger
+
+ # This module autodetects various platform-specific information, and
+@@ -263,15 +265,12 @@ def self.try_compile(language, source, flags = nil)
+ else
+ raise ArgumentError,"Unsupported language '#{language}'"
+ end
+- filename = File.join("#{tmpexedir}/passenger-compile-check-#{Process.pid}.c")
+- File.open(filename, "w") do |f|
+- f.puts(source)
+- end
+- begin
++ Dir.mktmpdir("passenger.", tmpexedir) do |dir|
++ filename = File.join(dir, "check.c")
++ File.open(filename, "w") do |f|
++ f.puts(source)
++ end
+ return system("(#{compiler} #{flags} -c '#{filename}' -o '#{filename}.o') >/dev/null 2>/dev/null")
+- ensure
+- File.unlink(filename) rescue nil
+- File.unlink("#{filename}.o") rescue nil
+ end
+ end
+ private_class_method :try_compile
+@@ -284,15 +283,12 @@ def self.try_link(language, source, flags = nil)
+ else
+ raise ArgumentError,"Unsupported language '#{language}'"
+ end
+- filename = File.join("#{tmpexedir}/passenger-link-check-#{Process.pid}.c")
+- File.open(filename, "w") do |f|
+- f.puts(source)
+- end
+- begin
++ Dir.mktmpdir("passenger.", tmpexedir) do |dir|
++ filename = File.join(dir, "check.c")
++ File.open(filename, "w") do |f|
++ f.puts(source)
++ end
+ return system("(#{compiler} #{flags} '#{filename}' -o '#{filename}.out') >/dev/null 2>/dev/null")
+- ensure
+- File.unlink(filename) rescue nil
+- File.unlink("#{filename}.out") rescue nil
+ end
+ end
+ private_class_method :try_link
+@@ -305,17 +301,16 @@ def self.try_compile_and_run(language, source, flags = nil)
+ else
+ raise ArgumentError,"Unsupported language '#{language}'"
+ end
+- filename = File.join("#{tmpexedir}/passenger-compile-check-#{Process.pid}.c")
+- File.open(filename, "w") do |f|
+- f.puts(source)
+- end
+- begin
++ Dir.mktmpdir("passenger.", tmpexedir) do |dir|
++ filename = File.join(dir, "check.c")
++ File.open(filename, "w") do |f|
++ f.puts(source)
++ end
+ if system("(#{compiler} #{flags} '#{filename}' -o '#{filename}.out') >/dev/null 2>/dev/null")
+ if Process.respond_to?(:spawn)
+ pid = Process.spawn("#{filename}.out",
+ :out => ["/dev/null", "w"],
+ :err => ["/dev/null", "w"])
+-
+ else
+ pid = fork do
+ STDOUT.reopen("/dev/null", "w")
+@@ -328,9 +323,6 @@ def self.try_compile_and_run(language, source, flags = nil)
+ else
+ return false
+ end
+- ensure
+- File.unlink(filename) rescue nil
+- File.unlink("#{filename}.out") rescue nil
+ end
+ end
+ private_class_method :try_compile_and_run
+--- a/lib/phusion_passenger/platform_info/apache.rb
++++ b/lib/phusion_passenger/platform_info/apache.rb
+@@ -285,16 +285,7 @@ def self.apu_libs
+ # headers are placed into the same directory as the Apache headers,
+ # and so 'apr-config' and 'apu-config' won't be necessary in that case.
+ def self.apr_config_needed_for_building_apache_modules?
+- filename = File.join("#{tmpexedir}/passenger-platform-check-#{Process.pid}.c")
+- File.open(filename, "w") do |f|
+- f.puts("#include <apr.h>")
+- end
+- begin
+- return !system("(gcc #{apache2_module_cflags(false)} -c '#{filename}' -o '#{filename}.o') >/dev/null 2>/dev/null")
+- ensure
+- File.unlink(filename) rescue nil
+- File.unlink("#{filename}.o") rescue nil
+- end
++ return !try_compile(:c, "#include <apr.h>", apache2_module_cflags(false))
+ end
+ memoize :apr_config_needed_for_building_apache_modules?
+
diff -Nru ruby-passenger-3.0.13debian/debian/patches/CVE-2013-4136.patch ruby-passenger-3.0.13debian/debian/patches/CVE-2013-4136.patch
--- ruby-passenger-3.0.13debian/debian/patches/CVE-2013-4136.patch 1970-01-01 01:00:00.000000000 +0100
+++ ruby-passenger-3.0.13debian/debian/patches/CVE-2013-4136.patch 2013-07-20 19:16:24.000000000 +0200
@@ -0,0 +1,152 @@
+Description: Fix for CVE-2013-4136: insecure tmp files usage
+Origin: backport, https://github.com/phusion/passenger/commit/5483b3292cc2af1c83033eaaadec20dba4dcfd9b
+ and https://github.com/phusion/passenger/commit/9dda49f4a3ebe9bafc48da1bd45799f30ce19566
+Bug: https://code.google.com/p/phusion-passenger/issues/detail?id=910
+Bug-Debian: http://bugs.debian.org/717176
+
+--- a/ext/common/LoggingAgent/Main.cpp
++++ b/ext/common/LoggingAgent/Main.cpp
+@@ -265,11 +265,6 @@
+ ev::sig sigtermWatcher(eventLoop);
+ ev::sig sigquitWatcher(eventLoop);
+
+- if (feedbackFdAvailable()) {
+- feedbackFdWatcher.set<&feedbackFdBecameReadable>();
+- feedbackFdWatcher.start(FEEDBACK_FD, ev::READ);
+- writeArrayMessage(FEEDBACK_FD, "initialized", NULL);
+- }
+ sigintWatcher.set<&caughtExitSignal>();
+ sigintWatcher.start(SIGINT);
+ sigtermWatcher.set<&caughtExitSignal>();
+@@ -281,6 +276,11 @@
+ /********** Initialized! Enter main loop... **********/
+
+ P_DEBUG("Logging agent online, listening at " << socketAddress);
++ if (feedbackFdAvailable()) {
++ feedbackFdWatcher.set<&feedbackFdBecameReadable>();
++ feedbackFdWatcher.start(FEEDBACK_FD, ev::READ);
++ writeArrayMessage(FEEDBACK_FD, "initialized", NULL);
++ }
+ ev_loop(eventLoop, 0);
+ return exitCode;
+ } catch (const tracable_exception &e) {
+--- a/ext/common/ServerInstanceDir.h
++++ b/ext/common/ServerInstanceDir.h
+@@ -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>
+@@ -38,6 +39,7 @@
+ #include <cstring>
+ #include <string>
+
++#include <Logging.h>
+ #include "Exceptions.h"
+ #include "Utils.h"
+ #include "Utils/StrIntUtils.h"
+@@ -47,6 +49,15 @@
+ using namespace std;
+ using namespace boost;
+
++/* TODO: I think we should move away from generation dirs in the future.
++ * That way we can become immune to existing-directory-in-tmp denial of
++ * service attacks. To achieve the same functionality as we do now, each
++ * server instance directory is tagged with the control process's PID
++ * and a creation timestamp. passenger-status should treat the server instance
++ * directory with the most recent creation timestamp as the one to query.
++ * For now, the current code does not lead to an exploit.
++ */
++
+ class ServerInstanceDir: public noncopyable {
+ public:
+ // Don't forget to update lib/phusion_passenger/admin_tools/server_instance.rb too.
+@@ -217,7 +228,69 @@
+ * 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=rwxs,g=rx,o=rx");
++ if (owner) {
++ switch (getFileType(path)) {
++ case FT_NONEXISTANT:
++ createDirectory(path);
++ break;
++ case FT_DIRECTORY:
++ verifyDirectoryPermissions(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);
++ }
++ // verifyDirectoryPermissions() checks for the owner/group so we must make
++ // sure the server instance directory has that owner/group, even when the
++ // parent directory has setgid on.
++ if (chown(path.c_str(), geteuid(), getegid()) == -1) {
++ int e = errno;
++ throw FileSystemException("Cannot change the permissions of the server "
++ "instance directory '" + path + "'", e, path);
++ }
++ }
++
++ /**
++ * 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) {
++ 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"))) {
++ 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:
++ * 1. Disabling user switching only lowers the privilege of the HelperAgent.
++ * 2. For the UID/GID to change, the web server must be completely restarted
++ * (not just graceful reload) so that the control process can change its UID/GID.
++ * This causes the PID to change, so that an entirely new server instance
++ * directory is created.
++ */
++ throw RuntimeException("Tried to reuse existing server instance directory " +
++ path + ", but it has wrong owner and group");
++ }
+ }
+
+ bool isDirectory(const string &dir, struct dirent *entry) const {
+--- a/test/cxx/ServerInstanceDirTest.cpp
++++ b/test/cxx/ServerInstanceDirTest.cpp
+@@ -73,9 +73,11 @@
+ }
+
+ 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(1234, parentDir, false);
+ ServerInstanceDir dir2(5678, parentDir);
diff -Nru ruby-passenger-3.0.13debian/debian/patches/series ruby-passenger-3.0.13debian/debian/patches/series
--- ruby-passenger-3.0.13debian/debian/patches/series 2013-05-30 09:48:17.000000000 +0200
+++ ruby-passenger-3.0.13debian/debian/patches/series 2013-07-20 10:56:30.000000000 +0200
@@ -1,3 +1,4 @@
fix_install_path.patch
fix_ftbfs_glibc217.patch
CVE-2013-2119.patch
+CVE-2013-4136.patch
More information about the Pkg-ruby-extras-maintainers
mailing list