Only accept actions without File::SEPARATOR in the name.

This will avoid directory traversal in implicit render.

Fixes: CVE-2014-0130

Conflicts:
	actionpack/lib/abstract_controller/base.rb

2 files changed, 41 insertions, 4 deletions

diff --git a/actionpack/lib/abstract_controller/base.rb b/actionpack/lib/abstract_controller/base.rb
index de14b59f8e..6d200a0333 100644
--- a/actionpack/lib/abstract_controller/base.rb
+++ b/actionpack/lib/abstract_controller/base.rb
@@ -127,7 +127,7 @@ module AbstractController
     def process(action, *args)
       @_action_name = action.to_s
 
-      unless action_name = method_for_action(@_action_name)
+      unless action_name = _find_action_name(@_action_name)
         raise ActionNotFound, "The action '#{action}' could not be found for #{self.class.name}"
       end
 
@@ -160,7 +160,7 @@ module AbstractController
     # ==== Returns
     # * <tt>TrueClass</tt>, <tt>FalseClass</tt>
     def available_action?(action_name)
-      method_for_action(action_name).present?
+      _find_action_name(action_name).present?
     end
 
     private
@@ -204,6 +204,23 @@ module AbstractController
       end
 
       # Takes an action name and returns the name of the method that will
+      # handle the action.
+      #
+      # It checks if the action name is valid and returns false otherwise.
+      #
+      # See method_for_action for more information.
+      #
+      # ==== Parameters
+      # * <tt>action_name</tt> - An action name to find a method name for
+      #
+      # ==== Returns
+      # * <tt>string</tt> - The name of the method that handles the action
+      # * false           - No valid method name could be found. Raise ActionNotFound.
+      def _find_action_name(action_name)
+        _valid_action_name?(action_name) && method_for_action(action_name)
+      end
+
+      # Takes an action name and returns the name of the method that will
       # handle the action. In normal cases, this method returns the same
       # name as it receives. By default, if #method_for_action receives
       # a name that is not an action, it will look for an #action_missing
@@ -225,7 +242,7 @@ module AbstractController
       #
       # ==== Returns
       # * <tt>string</tt> - The name of the method that handles the action
-      # * <tt>nil</tt>    - No method name could be found. Raise ActionNotFound.
+      # * <tt>nil</tt>    - No method name could be found.
       def method_for_action(action_name)
         if action_method?(action_name)
           action_name
@@ -233,5 +250,10 @@ module AbstractController
           "_handle_action_missing"
         end
       end
+
+      # Checks if the action name is valid and returns false otherwise.
+      def _valid_action_name?(action_name)
+        action_name.to_s !~ Regexp.new(File::SEPARATOR)
+      end
   end
 end
diff --git a/actionpack/test/controller/new_base/render_implicit_action_test.rb b/actionpack/test/controller/new_base/render_implicit_action_test.rb
index 1e2191d417..5b4885f7e0 100644
--- a/actionpack/test/controller/new_base/render_implicit_action_test.rb
+++ b/actionpack/test/controller/new_base/render_implicit_action_test.rb
@@ -6,7 +6,7 @@ module RenderImplicitAction
       "render_implicit_action/simple/hello_world.html.erb"     => "Hello world!",
       "render_implicit_action/simple/hyphen-ated.html.erb"     => "Hello hyphen-ated!",
       "render_implicit_action/simple/not_implemented.html.erb" => "Not Implemented"
-    )]
+    ), ActionView::FileSystemResolver.new(File.expand_path('../../../controller', __FILE__))]
 
     def hello_world() end
   end
@@ -33,10 +33,25 @@ module RenderImplicitAction
       assert_status 200
     end
 
+    test "render does not traverse the file system" do
+      assert_raises(AbstractController::ActionNotFound) do
+        action_name = %w(.. .. fixtures shared).join(File::SEPARATOR)
+        SimpleController.action(action_name).call(Rack::MockRequest.env_for("/"))
+      end
+    end
+
     test "available_action? returns true for implicit actions" do
       assert SimpleController.new.available_action?(:hello_world)
       assert SimpleController.new.available_action?(:"hyphen-ated")
       assert SimpleController.new.available_action?(:not_implemented)
     end
+
+    test "available_action? does not allow File::SEPARATOR on the name" do
+      action_name = %w(evil .. .. path).join(File::SEPARATOR)
+      assert_equal false, SimpleController.new.available_action?(action_name.to_sym)
+
+      action_name = %w(evil path).join(File::SEPARATOR)
+      assert_equal false, SimpleController.new.available_action?(action_name.to_sym)
+    end
   end
 end