Bug#726358: pu: package ruby-passenger/3.0.13debian-1+deb7u1
Package: release.debian.org
Severity: normal
Tags: wheezy
User: release.debian.org@packages.debian.org
Usertags: pu
There are two minor security issues in ruby-passenger:
CVE-2013-2119 and CVE-2013-4136: insecure tmp files usage
I'd like to fix those by backporting four upstream commits,
see the attached debdiff.
Cheers,
Felix
diff -Nru ruby-passenger-3.0.13debian/debian/changelog ruby-passenger-3.0.13debian/debian/changelog
--- ruby-passenger-3.0.13debian/debian/changelog 2012-06-28 17:00:51.000000000 +0200
+++ ruby-passenger-3.0.13debian/debian/changelog 2013-10-14 22:43:12.000000000 +0200
@@ -1,3 +1,11 @@
+ruby-passenger (3.0.13debian-1+deb7u1) wheezy; urgency=low
+
+ * Fix CVE-2013-2119 and CVE-2013-4136: insecure tmp files usage.
+ (Closes: #710351, #717176)
+ - Backport upstream commits in CVE-2013-2119.patch and CVE-2013-4136.patch
+
+ -- Felix Geyer <fgeyer@debian.org> Mon, 14 Oct 2013 22:43:07 +0200
+
ruby-passenger (3.0.13debian-1) unstable; urgency=low
* Team 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 1970-01-01 01:00:00.000000000 +0100
+++ ruby-passenger-3.0.13debian/debian/patches/CVE-2013-2119.patch 2013-07-20 13:37:26.000000000 +0200
@@ -0,0 +1,286 @@
+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
+
+--- a/bin/passenger-install-nginx-module
++++ b/bin/passenger-install-nginx-module
+@@ -27,6 +27,7 @@ $LOAD_PATH.unshift("#{passenger_root}/lib")
+ require 'phusion_passenger'
+ require 'optparse'
+ require 'fileutils'
++require 'tmpdir'
+ require 'phusion_passenger/platform_info/ruby'
+ require 'phusion_passenger/dependencies'
+ require 'phusion_passenger/abstract_installer'
+@@ -108,14 +109,12 @@ class Installer < PhusionPassenger::AbstractInstaller
+ def before_install
+ super
+ myself = `whoami`.strip
+- @working_dir = "/tmp/#{myself}-passenger-#{Process.pid}"
+- FileUtils.rm_rf(@working_dir)
+- FileUtils.mkdir_p(@working_dir)
++ @working_dir = Dir.mktmpdir("passenger.")
+ end
+
+ def after_install
+ super
+- FileUtils.rm_rf(@working_dir)
++ FileUtils.remove_entry_secure(@working_dir) if @working_dir
+ end
+
+ private
+--- a/lib/phusion_passenger/dependencies.rb
++++ b/lib/phusion_passenger/dependencies.rb
+@@ -22,6 +22,7 @@
+ # THE SOFTWARE.
+
+ require 'rbconfig'
++require 'tmpdir'
+ require 'phusion_passenger'
+ require 'phusion_passenger/packaging'
+ require 'phusion_passenger/platform_info'
+@@ -117,6 +118,12 @@ def self.mizuho_required?
+ end
+ end
+
++ def self.create_temp_files(name1, name2, dir = PlatformInfo.tmpexedir)
++ Dir.mktmpdir("passenger.", dir) do |subdir|
++ yield "#{subdir}/#{name1}", "#{subdir}/#{name2}"
++ end
++ end
++
+ GCC = Dependency.new do |dep|
+ dep.name = "GNU C++ compiler"
+ dep.define_checker do |result|
+@@ -456,9 +463,7 @@ def self.mizuho_required?
+ Curl_Dev = Dependency.new do |dep|
+ dep.name = "Curl development headers with SSL support"
+ dep.define_checker do |result|
+- source_file = "#{PlatformInfo.tmpexedir}/passenger-curl-check.c"
+- output_file = "#{PlatformInfo.tmpexedir}/passenger-curl-check"
+- begin
++ Dependencies.create_temp_files("check.c", "check") do |source_file, output_file|
+ found = true
+ File.open(source_file, 'w') do |f|
+ f.puts("#include <curl/curl.h>")
+@@ -482,9 +487,6 @@ def self.mizuho_required?
+ found = false
+ end
+ result.found(found)
+- ensure
+- File.unlink(source_file) rescue nil
+- File.unlink(output_file) rescue nil
+ end
+ end
+ dep.install_instructions = "Please download Curl from <b>http://curl.haxx.se/libcurl</b> " +
+@@ -514,22 +516,17 @@ def self.mizuho_required?
+ OpenSSL_Dev = Dependency.new do |dep|
+ dep.name = "OpenSSL development headers"
+ dep.define_checker do |result|
+- source_file = "#{PlatformInfo.tmpexedir}/passenger-openssl-check.c"
+- object_file = "#{PlatformInfo.tmpexedir}/passenger-openssl-check.o"
+- begin
++ Dependencies.create_temp_files("check.c", "check.o") do |source_file, output_file|
+ File.open(source_file, 'w') do |f|
+ f.write("#include <openssl/ssl.h>")
+ end
+ Dir.chdir(File.dirname(source_file)) do
+- if system("(gcc #{ENV['CFLAGS']} -c '#{source_file}') >/dev/null 2>/dev/null")
++ if system("(gcc #{ENV['CFLAGS']} -c '#{source_file}' -o '#{output_file}') >/dev/null 2>/dev/null")
+ result.found
+ else
+ result.not_found
+ end
+ end
+- ensure
+- File.unlink(source_file) rescue nil
+- File.unlink(object_file) rescue nil
+ end
+ end
+ if RUBY_PLATFORM =~ /linux/
+@@ -546,22 +543,17 @@ def self.mizuho_required?
+ Zlib_Dev = Dependency.new do |dep|
+ dep.name = "Zlib development headers"
+ dep.define_checker do |result|
+- source_file = "#{PlatformInfo.tmpexedir}/zlib-check.c"
+- object_file = "#{PlatformInfo.tmpexedir}/zlib-check.o"
+- begin
++ Dependencies.create_temp_files("check.c", "check.o") do |source_file, output_file|
+ File.open(source_file, 'w') do |f|
+ f.write("#include <zlib.h>")
+ end
+ Dir.chdir(File.dirname(source_file)) do
+- if system("(g++ -c zlib-check.c) >/dev/null 2>/dev/null")
++ if system("(g++ -c '#{source_file}' -o '#{output_file}') >/dev/null 2>/dev/null")
+ result.found
+ else
+ result.not_found
+ end
+ end
+- ensure
+- File.unlink(source_file) rescue nil
+- File.unlink(object_file) rescue nil
+ end
+ end
+ if RUBY_PLATFORM =~ /linux/
+--- 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)
+
+ def write_nginx_config_file
+ require 'phusion_passenger/platform_info/ruby'
+- ensure_directory_exists(@temp_dir)
+-
++ require 'tmpdir'
++ @temp_dir = Dir.mktmpdir("passenger.", "/tmp")
++ @config_filename = "#{@temp_dir}/config"
++ File.chmod(0755, @temp_dir)
++
+ File.open(@config_filename, 'w') do |f|
+ f.chmod(0644)
+ template_filename = File.join(TEMPLATES_DIR, "standalone", "config.erb")
+@@ -213,8 +216,6 @@ def nginx_ping_port
+ def create_nginx_controller(extra_options = {})
+ require_daemon_controller
+ require 'socket' unless defined?(UNIXSocket)
+- @temp_dir = "/tmp/passenger-standalone.#{$$}"
+- @config_filename = "#{@temp_dir}/config"
+ if @options[:socket_file]
+ ping_spec = [:unix, @options[:socket_file]]
+ else
+--- a/lib/phusion_passenger/standalone/runtime_installer.rb
++++ b/lib/phusion_passenger/standalone/runtime_installer.rb
+@@ -23,6 +23,7 @@
+ # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ # THE SOFTWARE.
+ require 'fileutils'
++require 'tmpdir'
+ require 'phusion_passenger'
+ require 'phusion_passenger/abstract_installer'
+ require 'phusion_passenger/packaging'
+@@ -164,16 +165,14 @@ def install!
+ def before_install
+ super
+ @plugin.call_hook(:runtime_installer_start, self) if @plugin
+- @working_dir = "/tmp/#{myself}-passenger-standalone-#{Process.pid}"
+- FileUtils.rm_rf(@working_dir)
+- FileUtils.mkdir_p(@working_dir)
++ @working_dir = Dir.mktmpdir("passenger.")
+ @download_binaries = true if !defined?(@download_binaries)
+ @binaries_url_root ||= STANDALONE_BINARIES_URL_ROOT
+ end
+
+ def after_install
+ super
+- FileUtils.rm_rf(@working_dir)
++ FileUtils.remove_entry_secure(@working_dir) if @working_dir
+ @plugin.call_hook(:runtime_installer_cleanup) if @plugin
+ end
+
+--- 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 2012-06-28 17:00:51.000000000 +0200
+++ ruby-passenger-3.0.13debian/debian/patches/series 2013-07-21 13:02:52.000000000 +0200
@@ -1 +1,3 @@
fix_install_path.patch
+CVE-2013-2119.patch
+CVE-2013-4136.patch
Reply to: