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
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 <fge...@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)" <hon...@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

Reply via email to