Package: release.debian.org Severity: normal Tags: wheezy User: release.debian....@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 <fge...@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