Openwall GNU/*/Linux - a small security-enhanced Linux distro for servers
[<prev] [next>] [day] [month] [year] [list]
Date: Thu, 30 Oct 2014 11:47:39 -0700
From: Aaron Patterson <tenderlove@...y-lang.org>
To: rubyonrails-security@...glegroups.com, oss-security@...ts.openwall.com,
	secalert@...hat.com
Subject: Arbitrary file existence disclosure in Action Pack (CVE-2014-7818)

Arbitrary file existence disclosure in Action Pack

There is an information leak vulnerability in Action Pack. This vulnerability
has been assigned the CVE identifier CVE-2014-7818.

Versions Affected:  >= 3.0.0
Not affected:       <= 3.0.0
Fixed Versions:     3.2.20, 4.0.11, 4.1.7, 4.2.0.beta3

Impact
------
Specially crafted requests can be used to determine whether a file exists on the filesystem that is outside the Rails application's root directory.  The files will not be served, but attackers can determine whether or not the file exists.

This only impacts Rails applications that enable static file serving at
runtime.  For example, the application's production configuration will say:

  config.serve_static_assets = true

All users running an affected release should either upgrade or use one of the work arounds immediately.

Releases 
-------- 
The 3.2.20, 4.0.11, 4.1.7 & 4.2.0.beta3 releases are available at the normal locations. 

Workarounds 
----------- 
To work around this issue, set config.serve_static_assets = false in an initializer.  This work around will not be possible in all hosting environments and upgrading is advised.

Patches 
------- 
To aid users who aren't able to upgrade immediately we have provided patches for the two supported release series.  They are in git-am format and consist of a single changeset. 

* 3-1-sec-static-files.patch - Patch for the 3.1.x release series
* 3-2-sec-static-files.patch - Patch for the 3.2.x release series
* 4-0-sec-static-files.patch - Patch for the 4.0.x release series
* 4-1-sec-static-files.patch - Patch for the 4.1.x release series

Please note that only the 3.2.x, 4.0.x & 4.1.x  series are supported at present.  Users of earlier unsupported releases are advised to upgrade as soon as possible as we cannot guarantee the continued availability of security fixes for unsupported releases.

Credits 
------- 

This vulnerability was reported by multiple researchers working independently.  Thanks to each of them for reporting the issue to us and verifying the fixes.

* Eaden McKee
* Dennis Hackethal & Christian Hansen of Crowdcurity
* Juan C. Mller & Mike McClurg of Greenhouse.io 
* Alex Ianus of Coinbase

-- 
Aaron Patterson
http://tenderlovemaking.com/

From 9c37d8eb8821aa8dd5ac395ae85344a63ae0e23c Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@...il.com>
Date: Fri, 10 Oct 2014 16:00:03 -0700
Subject: [PATCH] FileHandler should not be called for files outside the root

FileHandler#matches? should return false for files that are outside the
"root" path.

Conflicts:
	actionpack/lib/action_dispatch/middleware/static.rb

Conflicts:
	actionpack/lib/action_dispatch/middleware/static.rb
	actionpack/test/dispatch/static_test.rb
---
 actionpack/lib/action_dispatch/middleware/static.rb | 21 ++++++++++++++++++++-
 actionpack/test/dispatch/static_test.rb             | 18 ++++++++++++++++++
 2 files changed, 38 insertions(+), 1 deletion(-)

diff --git a/actionpack/lib/action_dispatch/middleware/static.rb b/actionpack/lib/action_dispatch/middleware/static.rb
index 31185fe..4c46283 100644
--- a/actionpack/lib/action_dispatch/middleware/static.rb
+++ b/actionpack/lib/action_dispatch/middleware/static.rb
@@ -11,7 +11,7 @@ module ActionDispatch
     def match?(path)
       path = path.dup
 
-      full_path = path.empty? ? @root : File.join(@...t, escape_glob_chars(unescape_path(path)))
+      full_path = path.empty? ? @root : File.join(@...t, escape_glob_chars(clean_path_info(unescape_path(path))))
       paths = "#{full_path}#{ext}"
 
       matches = Dir[paths]
@@ -40,6 +40,25 @@ module ActionDispatch
     def escape_glob_chars(path)
       path.gsub(/[*?{}\[\]]/, "\\\\\\&")
     end
+
+    private
+
+    PATH_SEPS = Regexp.union(*[::File::SEPARATOR, ::File::ALT_SEPARATOR].compact)
+
+    def clean_path_info(path_info)
+      parts = path_info.split PATH_SEPS
+
+      clean = []
+
+      parts.each do |part|
+        next if part.empty? || part == '.'
+        part == '..' ? clean.pop : clean << part
+      end
+
+      clean.unshift '/' if parts.empty? || parts.first.empty?
+
+      ::File.join(*clean)
+    end
   end
 
   class Static
diff --git a/actionpack/test/dispatch/static_test.rb b/actionpack/test/dispatch/static_test.rb
index 949deef..f225b02 100644
--- a/actionpack/test/dispatch/static_test.rb
+++ b/actionpack/test/dispatch/static_test.rb
@@ -136,10 +136,28 @@ class StaticTest < ActiveSupport::TestCase
     [200, {"Content-Type" => "text/plain"}, ["Hello, World!"]]
   }
   App = ActionDispatch::Static.new(DummyApp, "#{FIXTURE_LOAD_PATH}/public", "public, max-age=60")
+  Root = "#{FIXTURE_LOAD_PATH}/public"
 
   def setup
     @app = App
+    @root = Root
   end
 
   include StaticTests
+
+  def test_custom_handler_called_when_file_is_outside_root
+    filename = 'shared.html.erb'
+    assert File.exist?(File.join(@...t, '..', filename))
+    env = {
+      "REQUEST_METHOD"=>"GET",
+      "REQUEST_PATH"=>"/..%2F#{filename}",
+      "PATH_INFO"=>"/..%2F#{filename}",
+      "REQUEST_URI"=>"/..%2F#{filename}",
+      "HTTP_VERSION"=>"HTTP/1.1",
+      "SERVER_NAME"=>"localhost",
+      "SERVER_PORT"=>"8080",
+      "QUERY_STRING"=>""
+    }
+    assert_equal(DummyApp.call(nil), @app.call(env))
+  end
 end
-- 
1.9.3 (Apple Git-50)


From 83c32fb543f72732509a943523f9c7a25a44c07b Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@...il.com>
Date: Fri, 10 Oct 2014 16:00:03 -0700
Subject: [PATCH] FileHandler should not be called for files outside the root

FileHandler#matches? should return false for files that are outside the
"root" path.

Conflicts:
	actionpack/lib/action_dispatch/middleware/static.rb

Conflicts:
	actionpack/lib/action_dispatch/middleware/static.rb
	actionpack/test/dispatch/static_test.rb
---
 actionpack/lib/action_dispatch/middleware/static.rb | 21 ++++++++++++++++++++-
 actionpack/test/dispatch/static_test.rb             | 18 ++++++++++++++++++
 2 files changed, 38 insertions(+), 1 deletion(-)

diff --git a/actionpack/lib/action_dispatch/middleware/static.rb b/actionpack/lib/action_dispatch/middleware/static.rb
index a8d1765..7f11170 100644
--- a/actionpack/lib/action_dispatch/middleware/static.rb
+++ b/actionpack/lib/action_dispatch/middleware/static.rb
@@ -12,7 +12,7 @@ module ActionDispatch
     def match?(path)
       path = path.dup
 
-      full_path = path.empty? ? @root : File.join(@...t, escape_glob_chars(unescape_path(path)))
+      full_path = path.empty? ? @root : File.join(@...t, escape_glob_chars(clean_path_info(unescape_path(path))))
       paths = "#{full_path}#{ext}"
 
       matches = Dir[paths]
@@ -42,6 +42,25 @@ module ActionDispatch
       path.force_encoding('binary') if path.respond_to? :force_encoding
       path.gsub(/[*?{}\[\]]/, "\\\\\\&")
     end
+
+    private
+
+    PATH_SEPS = Regexp.union(*[::File::SEPARATOR, ::File::ALT_SEPARATOR].compact)
+
+    def clean_path_info(path_info)
+      parts = path_info.split PATH_SEPS
+
+      clean = []
+
+      parts.each do |part|
+        next if part.empty? || part == '.'
+        part == '..' ? clean.pop : clean << part
+      end
+
+      clean.unshift '/' if parts.empty? || parts.first.empty?
+
+      ::File.join(*clean)
+    end
   end
 
   class Static
diff --git a/actionpack/test/dispatch/static_test.rb b/actionpack/test/dispatch/static_test.rb
index 856746c..a035abd 100644
--- a/actionpack/test/dispatch/static_test.rb
+++ b/actionpack/test/dispatch/static_test.rb
@@ -140,10 +140,28 @@ class StaticTest < ActiveSupport::TestCase
     [200, {"Content-Type" => "text/plain"}, ["Hello, World!"]]
   }
   App = ActionDispatch::Static.new(DummyApp, "#{FIXTURE_LOAD_PATH}/public", "public, max-age=60")
+  Root = "#{FIXTURE_LOAD_PATH}/public"
 
   def setup
     @app = App
+    @root = Root
   end
 
   include StaticTests
+
+  def test_custom_handler_called_when_file_is_outside_root
+    filename = 'shared.html.erb'
+    assert File.exist?(File.join(@...t, '..', filename))
+    env = {
+      "REQUEST_METHOD"=>"GET",
+      "REQUEST_PATH"=>"/..%2F#{filename}",
+      "PATH_INFO"=>"/..%2F#{filename}",
+      "REQUEST_URI"=>"/..%2F#{filename}",
+      "HTTP_VERSION"=>"HTTP/1.1",
+      "SERVER_NAME"=>"localhost",
+      "SERVER_PORT"=>"8080",
+      "QUERY_STRING"=>""
+    }
+    assert_equal(DummyApp.call(nil), @app.call(env))
+  end
 end
-- 
1.9.3 (Apple Git-50)


From c47a4ae57b958c8e5b79d2b4636409f54d046d96 Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@...il.com>
Date: Fri, 10 Oct 2014 16:00:03 -0700
Subject: [PATCH] FileHandler should not be called for files outside the root

FileHandler#matches? should return false for files that are outside the
"root" path.

Conflicts:
	actionpack/lib/action_dispatch/middleware/static.rb
---
 .../lib/action_dispatch/middleware/static.rb       | 22 +++++++++++++++++++++-
 actionpack/test/dispatch/static_test.rb            | 22 ++++++++++++++++++++--
 2 files changed, 41 insertions(+), 3 deletions(-)

diff --git a/actionpack/lib/action_dispatch/middleware/static.rb b/actionpack/lib/action_dispatch/middleware/static.rb
index 2764584..df77021 100644
--- a/actionpack/lib/action_dispatch/middleware/static.rb
+++ b/actionpack/lib/action_dispatch/middleware/static.rb
@@ -14,7 +14,8 @@ module ActionDispatch
       path = unescape_path(path)
       return false unless path.valid_encoding?
 
-      full_path = path.empty? ? @root : File.join(@...t, escape_glob_chars(path))
+      full_path = path.empty? ? @root : File.join(@...t,
+        clean_path_info(escape_glob_chars(path)))
       paths = "#{full_path}#{ext}"
 
       matches = Dir[paths]
@@ -43,6 +44,25 @@ module ActionDispatch
     def escape_glob_chars(path)
       path.gsub(/[*?{}\[\]]/, "\\\\\\&")
     end
+
+    private
+
+    PATH_SEPS = Regexp.union(*[::File::SEPARATOR, ::File::ALT_SEPARATOR].compact)
+
+    def clean_path_info(path_info)
+      parts = path_info.split PATH_SEPS
+
+      clean = []
+
+      parts.each do |part|
+        next if part.empty? || part == '.'
+        part == '..' ? clean.pop : clean << part
+      end
+
+      clean.unshift '/' if parts.empty? || parts.first.empty?
+
+      ::File.join(*clean)
+    end
   end
 
   class Static
diff --git a/actionpack/test/dispatch/static_test.rb b/actionpack/test/dispatch/static_test.rb
index d3fe9df..79a7975 100644
--- a/actionpack/test/dispatch/static_test.rb
+++ b/actionpack/test/dispatch/static_test.rb
@@ -147,7 +147,8 @@ class StaticTest < ActiveSupport::TestCase
   }
 
   def setup
-    @app = ActionDispatch::Static.new(DummyApp, "#{FIXTURE_LOAD_PATH}/public", "public, max-age=60")
+    @root = "#{FIXTURE_LOAD_PATH}/public"
+    @app = ActionDispatch::Static.new(DummyApp, @root, "public, max-age=60")
   end
 
   def public_path
@@ -155,11 +156,28 @@ class StaticTest < ActiveSupport::TestCase
   end
 
   include StaticTests
+
+  def test_custom_handler_called_when_file_is_outside_root
+    filename = 'shared.html.erb'
+    assert File.exist?(File.join(@...t, '..', filename))
+    env = {
+      "REQUEST_METHOD"=>"GET",
+      "REQUEST_PATH"=>"/..%2F#{filename}",
+      "PATH_INFO"=>"/..%2F#{filename}",
+      "REQUEST_URI"=>"/..%2F#{filename}",
+      "HTTP_VERSION"=>"HTTP/1.1",
+      "SERVER_NAME"=>"localhost",
+      "SERVER_PORT"=>"8080",
+      "QUERY_STRING"=>""
+    }
+    assert_equal(DummyApp.call(nil), @app.call(env))
+  end
 end
 
 class StaticEncodingTest < StaticTest
   def setup
-    @app = ActionDispatch::Static.new(DummyApp, "#{FIXTURE_LOAD_PATH}/公共", "public, max-age=60")
+    @root = "#{FIXTURE_LOAD_PATH}/公共"
+    @app = ActionDispatch::Static.new(DummyApp, @root, "public, max-age=60")
   end
 
   def public_path
-- 
1.9.3 (Apple Git-50)


From 22ec7261be083ffaf8920b4c10e1e0504e26e186 Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@...il.com>
Date: Fri, 10 Oct 2014 16:00:03 -0700
Subject: [PATCH] FileHandler should not be called for files outside the root

FileHandler#matches? should return false for files that are outside the
"root" path.

Conflicts:
	actionpack/lib/action_dispatch/middleware/static.rb
---
 .../lib/action_dispatch/middleware/static.rb       | 22 +++++++++++++++++++++-
 actionpack/test/dispatch/static_test.rb            | 22 ++++++++++++++++++++--
 2 files changed, 41 insertions(+), 3 deletions(-)

diff --git a/actionpack/lib/action_dispatch/middleware/static.rb b/actionpack/lib/action_dispatch/middleware/static.rb
index 2764584..df77021 100644
--- a/actionpack/lib/action_dispatch/middleware/static.rb
+++ b/actionpack/lib/action_dispatch/middleware/static.rb
@@ -14,7 +14,8 @@ module ActionDispatch
       path = unescape_path(path)
       return false unless path.valid_encoding?
 
-      full_path = path.empty? ? @root : File.join(@...t, escape_glob_chars(path))
+      full_path = path.empty? ? @root : File.join(@...t,
+        clean_path_info(escape_glob_chars(path)))
       paths = "#{full_path}#{ext}"
 
       matches = Dir[paths]
@@ -43,6 +44,25 @@ module ActionDispatch
     def escape_glob_chars(path)
       path.gsub(/[*?{}\[\]]/, "\\\\\\&")
     end
+
+    private
+
+    PATH_SEPS = Regexp.union(*[::File::SEPARATOR, ::File::ALT_SEPARATOR].compact)
+
+    def clean_path_info(path_info)
+      parts = path_info.split PATH_SEPS
+
+      clean = []
+
+      parts.each do |part|
+        next if part.empty? || part == '.'
+        part == '..' ? clean.pop : clean << part
+      end
+
+      clean.unshift '/' if parts.empty? || parts.first.empty?
+
+      ::File.join(*clean)
+    end
   end
 
   class Static
diff --git a/actionpack/test/dispatch/static_test.rb b/actionpack/test/dispatch/static_test.rb
index afdda70..4e9cdc3 100644
--- a/actionpack/test/dispatch/static_test.rb
+++ b/actionpack/test/dispatch/static_test.rb
@@ -154,7 +154,8 @@ class StaticTest < ActiveSupport::TestCase
   }
 
   def setup
-    @app = ActionDispatch::Static.new(DummyApp, "#{FIXTURE_LOAD_PATH}/public", "public, max-age=60")
+    @root = "#{FIXTURE_LOAD_PATH}/public"
+    @app = ActionDispatch::Static.new(DummyApp, @root, "public, max-age=60")
   end
 
   def public_path
@@ -162,11 +163,28 @@ class StaticTest < ActiveSupport::TestCase
   end
 
   include StaticTests
+
+  def test_custom_handler_called_when_file_is_outside_root
+    filename = 'shared.html.erb'
+    assert File.exist?(File.join(@...t, '..', filename))
+    env = {
+      "REQUEST_METHOD"=>"GET",
+      "REQUEST_PATH"=>"/..%2F#{filename}",
+      "PATH_INFO"=>"/..%2F#{filename}",
+      "REQUEST_URI"=>"/..%2F#{filename}",
+      "HTTP_VERSION"=>"HTTP/1.1",
+      "SERVER_NAME"=>"localhost",
+      "SERVER_PORT"=>"8080",
+      "QUERY_STRING"=>""
+    }
+    assert_equal(DummyApp.call(nil), @app.call(env))
+  end
 end
 
 class StaticEncodingTest < StaticTest
   def setup
-    @app = ActionDispatch::Static.new(DummyApp, "#{FIXTURE_LOAD_PATH}/公共", "public, max-age=60")
+    @root = "#{FIXTURE_LOAD_PATH}/公共"
+    @app = ActionDispatch::Static.new(DummyApp, @root, "public, max-age=60")
   end
 
   def public_path
-- 
1.9.3 (Apple Git-50)



[ CONTENT OF TYPE application/pgp-signature SKIPPED ]

Powered by blists - more mailing lists

Your e-mail address:

Please check out the Open Source Software Security Wiki, which is counterpart to this mailing list.

Powered by Openwall GNU/*/Linux - Powered by OpenVZ