Fix Arbitrary file existence disclosure in Action Pack (CVE-2014-7818)

Genius · Oct 31, 2014 · 136a5ca · 136a5ca
1 parent a401afd
commit 136a5ca
Show file tree

Hide file tree

Showing 2 changed files with 34 additions and 4 deletions.
diff --git a/railties/lib/rails/rack/static.rb b/railties/lib/rails/rack/static.rb
@@ -15,13 +15,13 @@ def call(env)
         method      = env['REQUEST_METHOD']
 
         if FILE_METHODS.include?(method)
-          if file_exist?(path)
+          if file_exist_within_root?(path)
             return @file_server.call(env)
           else
             cached_path = directory_exist?(path) ? "#{path}/index" : path
             cached_path += ::ActionController::Base.page_cache_extension
 
-            if file_exist?(cached_path)
+            if file_exist_within_root?(cached_path)
               env['PATH_INFO'] = cached_path
               return @file_server.call(env)
             end
@@ -32,8 +32,26 @@ def call(env)
       end
 
       private
-        def file_exist?(path)
-          full_path = File.join(@file_server.root, ::Rack::Utils.unescape(path))
+
+        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
+
+        def file_exist_within_root?(path)
+          full_path = File.join(@file_server.root, clean_path_info(::Rack::Utils.unescape(path)))
           File.file?(full_path) && File.readable?(full_path)
         end
 

diff --git a/railties/test/rack_static_test.rb b/railties/test/rack_static_test.rb
@@ -5,12 +5,18 @@
 require 'fileutils'
 
 class RackStaticTest < ActiveSupport::TestCase
+
   def setup
+    @non_public_path = "#{RAILS_ROOT}/non_public_file.html"
     FileUtils.cp_r "#{RAILS_ROOT}/fixtures/public", "#{RAILS_ROOT}/public"
+    File.open(@non_public_path, 'w') do |file|
+      file.puts "non public content"
+    end
   end
 
   def teardown
     FileUtils.rm_rf "#{RAILS_ROOT}/public"
+    FileUtils.rm(@non_public_path)
   end
 
   DummyApp = lambda { |env|
@@ -40,6 +46,12 @@ def teardown
     assert_equal "/foo/index.html", get("/foo")
   end
 
+  test "does not betray the existance of files outside root" do
+    path = "../non_public_file.html"
+    assert File.exist?(File.join(RAILS_ROOT, 'public', path))
+    assert_equal get("/nofile"), get(path)
+  end
+
   private
     def get(path)
       Rack::MockRequest.new(App).request("GET", path).body