Skip to content
Permalink
Browse files Browse the repository at this point in the history
Fix writing a tmp file with a predictable name in
passenger-install-nginx-module.

With access to the system, a user could plant a symlink in /tmp that
resulted in a chosen-file overwrite attempt whenever
passenger-install-nginx-module was run, using the access rights of
the executing user, potentially even with chosen content.
  • Loading branch information
Daniel Knoppel (Phusion) committed Dec 5, 2016
1 parent f3ab65b commit e5b4b08
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 13 deletions.
1 change: 1 addition & 0 deletions CHANGELOG
Expand Up @@ -16,6 +16,7 @@ Next version (not yet released)
* Fixes an issue where passenger-config couldn't restart an app if the TMPDIR variable was set to /tmp
* `passenger-install-apache-module` now suggests the correct apache package on Ubuntu Xenial. Closes GH-1884.
* [Standalone] The TempDirToucher will now spend most of its time with reduced privileges, except when it's actively touching files. This allows it to be killed when Passenger is quit in most circumstances. Closes GH-1678.
* Fixes a file overwrite vulnerability caused by a predictable temporary file being written by `passenger-install-nginx-module`. Thanks to Jeremy Evans for reporting this.
* [Standalone] Fixes starting Passenger as a non-extant user. Closes GH-1849.
* Improved look of the error pages for failing to spawn an application (development & production mode), and Error ID is now also shown in production mode.
* [Standalone] Enable ipv6 support by default in builtin nginx. Closes GH-1873.
Expand Down
28 changes: 15 additions & 13 deletions bin/passenger-install-nginx-module
Expand Up @@ -38,11 +38,11 @@ PhusionPassenger.locate_directories
require 'digest/sha2'
require 'optparse'
require 'fileutils'
require 'tmpdir'
PhusionPassenger.require_passenger_lib 'platform_info/ruby'
PhusionPassenger.require_passenger_lib 'platform_info/openssl'
PhusionPassenger.require_passenger_lib 'abstract_installer'
PhusionPassenger.require_passenger_lib 'utils/terminal_choice_menu'
PhusionPassenger.require_passenger_lib 'utils/tmpio'
PhusionPassenger.require_passenger_lib 'utils/shellwords'

DOWNLOAD_OPTION = {
Expand Down Expand Up @@ -566,19 +566,21 @@ private

def pcre_is_installed?
if @pcre_is_installed.nil?
@pcre_is_installed = begin
File.open('/tmp/passenger-check.c', 'w') do |f|
f.puts("#include <pcre.h>")
end
Dir.chdir('/tmp') do
# Nginx checks for PCRE in multiple places...
system("(gcc -I/usr/local/include -I/usr/include/pcre " <<
"-I/usr/pkg/include -I/opt/local/include " <<
"-c passenger-check.c) >/dev/null 2>/dev/null")
Dir.mktmpdir do |safe_tmpdir|
@pcre_is_installed = begin
File.open("#{safe_tmpdir}/passenger-check.c", 'w') do |f|
f.puts("#include <pcre.h>")
end
Dir.chdir("#{safe_tmpdir}") do
# Nginx checks for PCRE in multiple places...
system("(gcc -I/usr/local/include -I/usr/include/pcre " <<
"-I/usr/pkg/include -I/opt/local/include " <<
"-c passenger-check.c) >/dev/null 2>/dev/null")
end
ensure
File.unlink("#{safe_tmpdir}/passenger-check.c") rescue nil
File.unlink("#{safe_tmpdir}/passenger-check.o") rescue nil
end
ensure
File.unlink('/tmp/passenger-check.c') rescue nil
File.unlink('/tmp/passenger-check.o') rescue nil
end
end
return @pcre_is_installed
Expand Down

2 comments on commit e5b4b08

@msmeissn
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have had Mitre assign CVE-2016-10345 to this issue.

@OnixGH
Copy link
Contributor

@OnixGH OnixGH commented on e5b4b08 Apr 19, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I've retroactively updated the blog post as well as the CHANGELOG.

Please sign in to comment.