Only write relative URIs when their normalized path begins with the n…

…ormalized cache directory path

[CVE-2020-8159]

lib/action_controller/caching/pages.rb

@@ -93,6 +93,10 @@ def cache_directory
             end
           end
 
+          def normalized_cache_directory
+            File.expand_path(cache_directory)
+          end
+
           def handle_proc_cache_directory
             if @controller
               @controller.instance_exec(&@cache_directory)
@@ -153,15 +157,22 @@ def cache_file(path, extension)
           end
 
           def cache_path(path, extension = nil)
-            File.join(cache_directory, cache_file(path, extension))
+            unnormalized_path = File.join(normalized_cache_directory, cache_file(path, extension))
+            normalized_path = File.expand_path(unnormalized_path)
+
+            relative_path if normalized_path.start_with?(normalized_cache_directory)
           end
 
           def delete(path)
+            return unless path
+
             File.delete(path) if File.exist?(path)
             File.delete(path + ".gz") if File.exist?(path + ".gz")
           end
 
           def write(content, path, gzip)
+            return unless path
+
             FileUtils.makedirs(File.dirname(path))
             File.open(path, "wb+") { |f| f.write(content) }
 

test/caching_test.rb

@@ -1,11 +1,13 @@
 require "abstract_unit"
 require "mocha/setup"
+require "find"
 
 CACHE_DIR = "test_cache"
 # Don't change "../tmp" cavalierly or you might hose something you don't want hosed
 TEST_TMP_DIR = File.expand_path("../tmp", __FILE__)
 FILE_STORE_PATH = File.join(TEST_TMP_DIR, CACHE_DIR)
 
+
 module PageCachingTestHelpers
   def setup
     super
@@ -175,6 +177,25 @@ class PageCachingTest < ActionController::TestCase
   include PageCachingTestHelpers
   tests PageCachingTestController
 
+  def test_cache_does_not_escape
+    draw do
+      get "/page_caching_test/ok/:id", to: "page_caching_test#ok"
+    end
+
+    project_root = File.expand_path("../../", __FILE__)
+
+
+    # Make a path that escapes the cache directory
+    get_to_root = "../../../"
+
+    # Make sure this relative path points at the project root
+    assert_equal project_root, File.expand_path(File.join(FILE_STORE_PATH, get_to_root))
+
+    get :ok, params: { id: "#{get_to_root}../pwnd" }
+
+    assert_predicate Find.find(File.join(project_root, "test")).grep(/pwnd/), :empty?
+  end
+
   def test_page_caching_resources_saves_to_correct_path_with_extension_even_if_default_route
     draw do
       get "posts.:format", to: "posts#index", as: :formatted_posts