Merge branch 'dir-traversal' into 'master'

Fix directory traversal vulnerabilities

Fixes gitlab/gitlab-ee#272.

As @joern mentions:

> This is not exploitable via the front-end nginx. But nevertheless this issue should be addressed.

See merge request !1760

1 files changed, 32 insertions, 1 deletions

diff --git a/app/controllers/help_controller.rb b/app/controllers/help_controller.rb
index fbd9e67e6df..0e5567c7734 100644
--- a/app/controllers/help_controller.rb
+++ b/app/controllers/help_controller.rb
@@ -3,7 +3,7 @@ class HelpController < ApplicationController
   end
 
   def show
-    @filepath = params[:filepath]
+    @filepath = clean_path_info(params[:filepath])
     @format = params[:format]
 
     respond_to do |format|
@@ -36,4 +36,35 @@ class HelpController < ApplicationController
 
   def ui
   end
+
+  PATH_SEPS = Regexp.union(*[::File::SEPARATOR, ::File::ALT_SEPARATOR].compact)
+
+  # Taken from ActionDispatch::FileHandler
+  # Cleans up the path, to prevent directory traversal outside the doc folder.
+  def clean_path_info(path_info)
+    parts = path_info.split(PATH_SEPS)
+
+    clean = []
+
+    # Walk over each part of the path
+    parts.each do |part|
+      # Turn `one//two` or `one/./two` into `one/two`.
+      next if part.empty? || part == '.'
+
+      if part == '..'
+        # Turn `one/two/../` into `one`
+        clean.pop
+      else
+        # Add simple folder names to the clean path.
+        clean << part
+      end
+    end
+
+    # If the path was an absolute path (i.e. `/` or `/one/two`),
+    # add `/` to the front of the clean path.
+    clean.unshift '/' if parts.empty? || parts.first.empty?
+
+    # Join all the clean path parts by the path separator.
+    ::File.join(*clean)
+  end
 end